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

Fixing url parse errors #173

Open
jameslzhu opened this issue Jan 25, 2018 · 2 comments
Open

Fixing url parse errors #173

jameslzhu opened this issue Jan 25, 2018 · 2 comments
Labels

Comments

@jameslzhu
Copy link
Member

jameslzhu commented Jan 25, 2018

Bug reproduction: navigate to any url of the form:

These are available on Rollbar as error 68, 93, 95, 99, 110.

These all stem from poor url parsing, especially assumed Integer conversion successes (errors are uncaught).

Ex: error 68.

Traceback: models/exam.rb.

Root cause: department (decoded from url) assumed to be found (no nil checks), causing dept.name to throw 'undefined method 'name''.

@jameslzhu jameslzhu added the bug label Jan 25, 2018
@jameslzhu
Copy link
Member Author

It appears that, for many invalid routes below the root, the 404 page isn't used; instead handlers frequently assume that the url is valid, and throw some sort of error when this isn't the case.

I don't think it's feasible to handle 404 errors at the top-level router (as response handling is delegated), but perhaps for each subpage's routing it may be.

@jameslzhu jameslzhu changed the title Bad exam archive url 500s instead of 404s Fixing url parse errors Jan 30, 2019
@jameslzhu
Copy link
Member Author

Update: https://rollbar.com/compserv/hkn-rails/items/93/?person_page=0&#ip-addresses
This contained the following error traceback:

ArgumentError: invalid value for Integer(): "/../../../../../../../windows/win.ini............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................\u0000."

File /home/h/hk/hkn/hkn-rails/migrate/releases/20190107053052/app/controllers/events_controller.rb, line 62 in index

The root of this is bad url parsing: assuming a cast to Integer will work, and not handling any errors.

@jameslzhu jameslzhu mentioned this issue Feb 8, 2019
jvperrin added a commit that referenced this issue Aug 13, 2019
These have been noisy and not actionable, so let's fix them so we stop
getting spammed via email about them

Fixes #197 (and helps with some of #173)

Note that some URIs still do produce errors, but it's much fewer than
before anyway
jameslzhu pushed a commit that referenced this issue Aug 20, 2019
These have been noisy and not actionable, so let's fix them so we stop
getting spammed via email about them

Fixes #197 (and helps with some of #173)

Note that some URIs still do produce errors, but it's much fewer than
before anyway
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant