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

Don't urldecode bso_ids from JSON #764

Closed
pjenvey opened this issue Aug 5, 2020 · 0 comments · Fixed by #792
Closed

Don't urldecode bso_ids from JSON #764

pjenvey opened this issue Aug 5, 2020 · 0 comments · Fixed by #792
Assignees
Labels
2 Estimate - s - This is a small change with clearly defined parameters. bug Something isn't working

Comments

@pjenvey
Copy link
Member

pjenvey commented Aug 5, 2020

Follow up to #744 (sorry I missed this before it was merged): we don't want to urldecode bso_ids pulled from JSON payloads, as there's no escaping done there so they should come in unmodified.

We only want to urldecode values received from url path elements.

Another thought: I don't think we need the extra is_ascii check (and the associated InvalidSubmission) -- because all 3 values we pull from the path (bso_id, collection, and uid) do their own validation. The first 2 via a REGEX and uid via u64::from_str.

To further enforce validation, if anything, we could also:

  • Make sure these results are always validated. Forcing all callers through extrude ensures so. We mistakenly left BsoParam::bsoparam_from_path as a pub function. If mistakenly called instead of extrude the caller could get an unvalidated BsoParam.
  • u64::from does recognize a leading '+', any source of funny business here with the auth token's contents (probably not)?
@pjenvey pjenvey added the bug Something isn't working label Aug 5, 2020
@pjenvey pjenvey added the 2 Estimate - s - This is a small change with clearly defined parameters. label Aug 10, 2020
@pjenvey pjenvey self-assigned this Aug 18, 2020
pjenvey added a commit that referenced this issue Aug 18, 2020
- add a keepalive setting
- fix: don't urldecode bso_ids from JSON
- pass the user-agent to sentry as an extra

Closes #786
Closes #785
Closes #764
Closes #787
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 Estimate - s - This is a small change with clearly defined parameters. bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant