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

Merged
merged 1 commit into from Jun 27, 2019
Merged

Half-fix of #96 #119

merged 1 commit into from Jun 27, 2019

Conversation

qwazix
Copy link
Collaborator

@qwazix qwazix 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

@thebaer
Copy link
Member

thebaer 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
Copy link

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

@qwazix
Copy link
Collaborator Author

qwazix 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.

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
Copy link
Collaborator Author

qwazix 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 :-/ )

@thebaer
Copy link
Member

thebaer commented Jun 27, 2019

Tested and verified this works. Thanks!

@thebaer thebaer merged commit 00a8f8c into writefreely:develop Jun 27, 2019
@thebaer thebaer added this to the 0.10 milestone Jun 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants