Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #569 - Manually encode Uri query parameters #596

Merged
merged 3 commits into from
Jan 14, 2021

Conversation

sloshy
Copy link
Contributor

@sloshy sloshy commented Jan 13, 2021

Fixes #569. The solution to the issue of query parameters being encoded incorrectly lies within a recently-added workaround in http4s, Query.fromString, which preserves the string you pass to it without trying to apply its own encoding rules.

Please let me know if the tests could be improved or if there is more documentation required.

@github-actions github-actions bot added the bug Something isn't working label Jan 13, 2021
@sloshy sloshy force-pushed the fix/569-issues-url-encoding branch from d40bbaa to 2ec3411 Compare January 13, 2021 04:57
@sloshy
Copy link
Contributor Author

sloshy commented Jan 13, 2021

Tests are failing, partial fix is in #595 (review appreciated) but the tests for this that were added pass.

@sloshy sloshy force-pushed the fix/569-issues-url-encoding branch from 2ec3411 to 5c61416 Compare January 13, 2021 05:05
@sloshy sloshy force-pushed the fix/569-issues-url-encoding branch from 5c61416 to aefdf2b Compare January 13, 2021 05:09
Copy link
Member

@juanpedromoreno juanpedromoreno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sloshy ! LGTM

Copy link
Contributor

@BenFradet BenFradet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍

@codecov
Copy link

codecov bot commented Jan 14, 2021

Codecov Report

Merging #596 (084852a) into master (fd7154e) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #596      +/-   ##
==========================================
+ Coverage   90.18%   90.22%   +0.04%     
==========================================
  Files          25       25              
  Lines         611      614       +3     
  Branches        1        4       +3     
==========================================
+ Hits          551      554       +3     
  Misses         60       60              
Impacted Files Coverage Δ
...4s/src/main/scala/github4s/http/Http4sSyntax.scala 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd7154e...084852a. Read the comment docs.

@sloshy sloshy merged commit 739bb97 into master Jan 14, 2021
@sloshy sloshy deleted the fix/569-issues-url-encoding branch January 14, 2021 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Receive 422 response when searching Issues
3 participants