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

[BUG] Fix bad use of strripos #206

Merged
merged 1 commit into from
Apr 25, 2023
Merged

[BUG] Fix bad use of strripos #206

merged 1 commit into from
Apr 25, 2023

Conversation

auroraeosrose
Copy link
Contributor

It is 100% valid for strripos to return 0 and still be considered a match
we want quotes in that case for the AS blah
NEVER EVER if a strripos, always explicitly check for false!!

This was breaking jsonb text returns with mixed case aliases

Select "pt"."name"->>'en_US' AS "myDescription" from mytable pt is what we want to end up with

The replaceNamesAndAliasIn was getting ' AS myDescription' and not properly escaping it because the strripos was returning 0

It is 100% valid for strripos to return 0
we want quotes in that case for the AS blah
NEVER EVER if a strripos, always explicitly check for false!!

This was breaking jsonb text returns with mixed case aliases

Select "pt"."name"->>'en_US' AS "myDescription" from mytable pt is what we want to end up with
The replaceNamesAndAliasIn was getting ' AS myDescription' and not properly escaping it because the strripos was returning 0
@harikt
Copy link
Member

harikt commented Apr 25, 2023

Hey @auroraeosrose ,

Thank you for the PR.

Can I add you to one of the maintainers. So you can also merge and release soon. I didn't find this earlier or missed for some reason. So don't want PR to wait so long.

@harikt harikt merged commit 8dc7bb6 into auraphp:4.x Apr 25, 2023
@harikt
Copy link
Member

harikt commented Apr 25, 2023

@koriym I guess I made a mistake. We never released 4.x . So merging to 4.x was something I did wrong. I guess it should go to 3.x.

@auroraeosrose
Copy link
Contributor Author

Should probably go to both if the issue exists in both branches
I'd have to look at the code :)

@harikt
Copy link
Member

harikt commented Apr 25, 2023

I have pushed to 3.x, 2.x . 4.x seems not released yet.

Bug fixes are released in 2.8.1 and 3.0.0 .

Thank you.

@koriym
Copy link
Member

koriym commented Apr 25, 2023

@harikt Yes. The 3.x test covers PHP 5.6 through PHP 8.2. (We don't need the 4.x branch now, right?)

@harikt
Copy link
Member

harikt commented Apr 26, 2023

Cool. Thank you.

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.

3 participants