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 "Location" header url corrupted by percent-unescaping in redirect… #1459

Closed
wants to merge 2 commits into from

Conversation

maudoin
Copy link

@maudoin maudoin commented Dec 27, 2022

Redirected GET request like https://sphinx.acast.com/p/acast/s/a-bientot-de-te-revoir/e/63a4721c69c77e001126ad39/media.mp3
are failing and while it looks like an agent setup problem it is actually the processing of the Location response header.

httplib is percent-un-escaping every response header with decode_url in parse_header, giving this address
https://stitcher2.acast.com/livestitches/4d4cc4fe72c9452bd0b0992a5c89e434.mp3?aid=63a4721c69c77e001126ad39&chid=a8879bdf-de58-4537-8dab-a3bb13948786&ci=oFpQlSRp3GFDZwrcZw5e3SEuFWtGfBXjcj6-mtxC8TJYKWNWTP3KWg==&pf=rss&sv=sphinx@1.134.1&uid=6ec01abdba610f88f88e42ff560ecdef&Expires=1672100680731&Key-Pair-Id=K38CTQXUSD0VVB&Signature=XoIMT7YfpbpOerJXwA4JVT-zat8V2flxU5AKtwr8LEGegGAu6hNSgeyLgq7gQmpv6pv6im2hKSyfUUqQmBEW8MCFLUYiUXuSVEcuVZ3BAT8u0gzcSdTFC1wOGhZTAExH15vei9-UAOVMj7Mq-jP-8hd-H~Atrj2YKI9krbWoslScK4yepWvpzwvBWP8-58NPIy6FaSfMHWwODigNCrJudiR0DPrr6x-HVSiwB~q5aTNVvlABQqGxNkpWtnAie8TuYKEvmioTlEL1aFj8RxMWke7yRc4uOchJtak5COoej4x780f0mepp-eh0OGtsB1izB7hGsyob0c8DwCYoVGTsRg__ in the Location result header and the redirected request fails.

However Firefox (for instance) successfully redirects to
https://stitcher2.acast.com/livestitches/4d4cc4fe72c9452bd0b0992a5c89e434.mp3?aid=63a4721c69c77e001126ad39&chid=a8879bdf-de58-4537-8dab-a3bb13948786&ci=oFpQlSRp3GFDZwrcZw5e3SEuFWtGfBXjcj6-mtxC8TJYKWNWTP3KWg%3D%3D&pf=rss&sv=sphinx%401.134.1&uid=6ec01abdba610f88f88e42ff560ecdef&Expires=1672109987714&Key-Pair-Id=K38CTQXUSD0VVB&Signature=TQXsBs7XluU~YRtPTcYe1EtVuvnkf542tbp1p7KUnvn24rm-tQjO8dYgLSbXlJCBwsiPtbnJc-YjLbGlaVLKDzzfABj2lCldE-KoeUSdnEQPWXdPK6FK5BR7kuN-CuY1MfQ-0sDa4MTGAErHZZB1p3~jiiZbbP7fYd9ttBfXwlZgjv5BtHOL4KQs7QY7q-~ZP5tXoGhtufPMruWRYOptrves991ax5lgKPwTvzhXSL6CEKpHWoAMi88shXnBBC~f2iOropB-yzcj5K-uaK6LPcObfHh9Akgl~uIAqbLka2Nrq-HQ-7QrMIUmFcA2nTEaAF66dGRj7AGtEkS2m2hB4A__, with escaped parts here %3D%3D&pf=rss&sv=sphinx%40

This fix skips decoding for Location (case sensitive) only, it may be too restrictive.

@yhirose
Copy link
Owner

yhirose commented Dec 27, 2022

@maudoin, thanks for the pull request. As I always ask any contributors, could you add a unit text to verify your change is good? Thanks.

@maudoin
Copy link
Author

maudoin commented Dec 28, 2022

@yhirose I will try to do this this week, I don't have a working supported environment yet (I dropped the header in a cmake/mingw project that doesn't have gtest)

@maudoin
Copy link
Author

maudoin commented Dec 30, 2022

@yhirose, here is an update with a test for this specific Location

@yhirose
Copy link
Owner

yhirose commented Dec 31, 2022

@yhirose could you format your code by using clangformat with .clang-format in this repo?

@yhirose
Copy link
Owner

yhirose commented Feb 17, 2023

@maudoin can I ask you a question?

This fix skips decoding for Location (case sensitive) only, it may be too restrictive.

What does it mean? HTTP header key is case-insensitive.
https://stackoverflow.com/questions/5258977/are-http-headers-case-sensitive

Thanks!

@maudoin
Copy link
Author

maudoin commented Feb 17, 2023

Indeed for the website I tested with I received a header with a capital L and it was enough to fix for me. I was not sure if I should have converted both the constant and the variable to lowercase to be case insensitive, looking at your link I should have...

@yhirose
Copy link
Owner

yhirose commented Mar 10, 2023

@maudoin sorry for the delay in responding. I have one more question. You mentioned FireFox handles it properly. How about curl and other brothers like Chrome or Edge?

@maudoin
Copy link
Author

maudoin commented Mar 11, 2023

the original link expired but you can also try

https://sphinx.acast.com/p/acast/s/a-bientot-de-te-revoir/e/63ebb88102ceee0011d61116/media.mp3

it works in opera, edge and chrome as well where it is redirected with an initial code 302 through the location header to

https://stitcher2.acast.com/livestitches/a5d2ed841fb4ab95b5b506346c279a57.mp3?aid=63ebb88102ceee0011d61116&chid=a8879bdf-de58-4537-8dab-a3bb13948786&ci=cZrHefNW8vl-9TIIlHQLGiYVcJYf8tq-JS5sFxQA4aEYq6TNi86LyA==&pf=rss&sv=sphinx@1.153.1&uid=0376c7c2f1a02cb4108bf0f77c19d200&Expires=1678505407834&Key-Pair-Id=K38CTQXUSD0VVB&Signature=c5re1JRDWZKR3guSRB31Ui1yDq5VMDMI6YUn~TxJCREgx0VgVXREyhK8Ioc-kSVeoJzo2dKPspRZB5WFbuqSKiOtxu~1AlU6UzWArWW8Ufh6i5dmyeCbOGEFiW586Xa6KYCpYYlUAsnIXrh7jALfVeEDaE13pFf3uNB78xEpLabcVP4IdPyndqB50FAACcTCOgqMkyqY2x~e6UAJzURLgEOshoQ6wAAJ3QQ6suo~HRY5rNoBCFSEIHpC7-c7AlphLJo3eo9EyVk1yKvJ1-DX3L1j3loVPs3G8Cy-rb4oYuYKdDDaogclR34Hc~qtYBj7bvOeeHcufz36gFv6~aj03Q__

on the other hand
curl https://sphinx.acast.com/p/acast/s/a-bientot-de-te-revoir/e/63ebb88102ceee0011d61116/media.mp3
gives me
curl: (35) schannel: next InitializeSecurityContext failed: Unknown error (0x80092012) - The revocation function was unable to check revocation for the certificate.
so I am not sure

yhirose added a commit that referenced this pull request Mar 11, 2023
@yhirose yhirose closed this in 9bb3ca8 Mar 11, 2023
@yhirose
Copy link
Owner

yhirose commented Mar 11, 2023

@maudoin, could you try with the latest httplib.h in the master branch? It fixes the issue that you reported and another related problem in the server as well.

ExclusiveOrange pushed a commit to ExclusiveOrange/cpp-httplib-exor that referenced this pull request May 2, 2023
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

Successfully merging this pull request may close these issues.

2 participants