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

Go back to uppercase URI encoding #83

Closed
DanBurton opened this issue Jan 25, 2018 · 5 comments
Closed

Go back to uppercase URI encoding #83

DanBurton opened this issue Jan 25, 2018 · 5 comments

Comments

@DanBurton
Copy link

The URI spec says:

For consistency, URI producers and
normalizers should use uppercase hexadecimal digits for all percent-
encodings.

Given how widely used http-types is in the Haskell ecosystem, I think it would be wise to follow the spec's recommendation on this.

Additionally, please note that we may keep http-types constrained to 0.10 on Stackage, due to 0.11's adverse effect on amazonka request signing (see brendanhay/amazonka#440) and other such packages.

See also: commercialhaskell/stackage#3226

@aristidb
Copy link
Owner

Thanks for pointing this out, I was a little bit uneasy changing the case, but was at the time happy that using bytestring's hexadecimal function simplified the code.

If this breaks request signing of amazonka, I will definitely revert back to upper-case.

@aristidb
Copy link
Owner

@DanBurton Would you mind checking if the latest change would fix your issue? Also, is it good for you if I release this as 0.12 (it is technically a breaking change compared to 0.11...)?

@DanBurton
Copy link
Author

is it good for you if I release this as 0.12?

It will take some time for everyone to bump their upper bounds as usual, but that's fine by me.

Would you mind checking if the latest change would fix your issue?

Sure, I'll give it a try and let you know how it goes.

@DanBurton
Copy link
Author

I have tested the following:

  • amazonka-ec2-1.5.0
  • amazonka-s3-1.5.0
  • amazonka-sqs-1.5.0

For each, I used the following stack.yaml

resolver: nightly-2018-01-25
packages:
- .
extra-deps:
- git: git@github.com:aristidb/http-types
  commit: 86ef7fea760c767cb4a8c8bb917614be49a0306c

And ran

$ stack unpack $pkg
$ cd $pkg
$ vi stack.yaml # add the contents above
$ stack test

The test suite passed for each one. I have reasonable confidence that the latest change (commit 86ef7fe) did in fact fix this issue.

@aristidb
Copy link
Owner

0.12 is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants