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

Preserve dots in parameters in RequestQueryHelper. Fixes #236 #237

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

neildaniels
Copy link
Contributor

@neildaniels neildaniels commented Apr 16, 2021

parse_str converts periods/dots in parameters to underscores, which ruins TMDb keys such as air_date_gte

This is a naive implementation of query string parsing, but hopefully is adequate in this context.

@neildaniels neildaniels marked this pull request as draft April 16, 2021 20:06
@neildaniels neildaniels marked this pull request as ready for review April 16, 2021 20:28
@wtfzdotnet
Copy link
Member

Sorry for the late pickup; just noticed the ticket ( I am not on github much within my current work environment ), good catch of a never catched "edge-case" :-). I'm not sure if I agree with the way the problem is being solved though, gonna set up to play with this in a bit.

@wtfzdotnet
Copy link
Member

wtfzdotnet commented Apr 26, 2021

@neildaniels given I didn't feel like I would have agreed with this solution to fix #236, I created a unit test to verify the issue exists to begin with, however either the problem lies elsewhere, or I just don't understand how to reproduce this issue.

Could you rebase on the latest 4.0 branch and modify the change introduced with ef9a246 to validate the case actually fails?

@wtfzdotnet
Copy link
Member

@neildaniels disregard my last message; whilst tinkering more and more I was able to reproduce

@neildaniels
Copy link
Contributor Author

@wtfzdotnet Can this get merged in?

@wtfzdotnet
Copy link
Member

Sorry @neildaniels , I'll take a look after work in like 5 hours.

@wtfzdotnet wtfzdotnet merged commit 88f7dd9 into php-tmdb:4.0 Aug 18, 2021
@wtfzdotnet
Copy link
Member

@neildaniels I'll get back to this at another time (and see if I can find a fix that feels better to me), github isn't really on my radar so do tag me like that if it takes a while :-)... Distracted with other projects and work, busy life!

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.

RequestQueryHelper unexpectedly converts dots/periods in parameters to underscores
2 participants