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

Case Sensitivity in URI segements #12

Open
reevesc opened this issue Sep 25, 2017 · 2 comments
Open

Case Sensitivity in URI segements #12

reevesc opened this issue Sep 25, 2017 · 2 comments

Comments

@reevesc
Copy link

reevesc commented Sep 25, 2017

Looks like there's a small bug in the query logic of the set_category_segments() method. I actually encountered the issue on an older version of the extension (1.0.5) but tested on the lastest version and it's still there.

Details:

On line 209 (version 1.12.0) the where_in() method is called and the $segs array is passed in, but the query that's generated is not case sensitive. So, if there is a category with a cat_url_title of 'hello' and one of the URI segments is 'HELLO' - the query returns a matching result. By itself, that's not really an issue, but when the $ids array is flipped (line 216) PHP throws an undefined index warning because it's trying to get the value of $ids['hello'] which doesn't exist (it should be looking for $ids['HELLO']) on lines 221-226.

I was able to patch the issue by changing line 209 from:

->where_in('cat_url_title, $segs)`

To:

->where_in('CAST(cat_url_title as BINARY)', $segs)

Looks like there are several ways to address it, the one above seems to be working quite well.
That said, the site does not have Publisher installed so I'm not able to test against that module (I didn't dive too deep into the logic so it may not even be relevant).

Let me know if you need more info or would like me to send up a pull request.

C.

@litzinger
Copy link
Owner

Just now getting around to this. Yeah that change makes sense. I'll merge the PR if you submit one.

@reevesc
Copy link
Author

reevesc commented Oct 16, 2017

Will do.

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