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

Half-fix of #96 #119

Open
wants to merge 1 commit into
base: develop
from

Conversation

Projects
None yet
3 participants
@qwazix
Copy link

commented Jun 9, 2019

This solves the error 500 on the /api/me endpoint but the issue is
not completely solved as this now throws a 401, similar to the other
endpoints, due to SQLite returning an empty result set when queried
with byte array. See the issue discussion for more info.


  • I have signed the CLA

@qwazix qwazix force-pushed the qwazix:develop branch from 622d4a8 to c16d915 Jun 11, 2019

@thebaer

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

Thanks for taking a stab at this. Based on this Stack Overflow answer, I guess LIKE compares character by character, where = does not, so that's why this works. Great catch.

As for the db.now() fixes, does this still work on MySQL? I haven't tested yet, but I believe that having the db.now() call as a parameter instead of inline the in query string will mean that it gets converted to text and escaped. E.g. it becomes expires > "NOW()" instead of expires > NOW()

@leo60228

This comment has been minimized.

Copy link

commented Jun 11, 2019

I'm using the latest commit in this PR and I can login, but all authenticated endpoints fail.

@qwazix qwazix force-pushed the qwazix:develop branch from 91c93cd to 0119b41 Jun 11, 2019

@qwazix

This comment has been minimized.

Copy link
Author

commented Jun 11, 2019

There were a few more places where I should replace = with LIKE. I still need to check if MySQL expires tokens correctly.

@qwazix qwazix force-pushed the qwazix:develop branch from 9d1e9d3 to 430ff30 Jun 11, 2019

Michael Demetriou
Fix #96
This solves the error 500 on the /api/me endpoint.

Replace token search query `=` with `LIKE` to fix sqlite complaining about
no valid tokens. Also checked with MySQL and it still works after the change.

@qwazix qwazix force-pushed the qwazix:develop branch from 430ff30 to 9570388 Jun 11, 2019

@qwazix

This comment has been minimized.

Copy link
Author

commented Jun 11, 2019

As for the db.now() fixes, does this still work on MySQL? I haven't tested yet, but I believe that having the db.now() call as a parameter instead of inline the in query string will mean that it gets converted to text and escaped. E.g. it becomes expires > "NOW()" instead of expires > NOW()

You were right. I replaced the ? substitution with string concatenation and tested both MySQL and SQLite with expired tokens and both work as expected.

Pls review.

(Btw I squashed all the commits because I had made a mess of them so you'll have to fetch again, sorry :-/ )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.