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

issue #420 return 400 response when Max-Days is too big #422

Merged
merged 4 commits into from
Oct 25, 2021

Conversation

kugiyasan
Copy link
Contributor

No description provided.

@aspacca
Copy link
Collaborator

aspacca commented Oct 16, 2021

@kugiyasan thanks for trying to solve the issue, but this doesn't seem the proper solution.
I still have to understand what's exactly the bug: and integer overflow or a problem in the check for the max date constraint.

your code doesn't fix either of them: also the metadata are saved anyway even if the max date is "wrong"

@kugiyasan
Copy link
Contributor Author

Oh sorry, I jumped the gun and forgot to explain!

I played around with a local server, and Max-Days: 100000 is valid, while Max-Days: 110000 is invalid. Looking at the source code, this header is converted to an int and is then passed to time.Duration. time.Duration's documentation says that:

The representation limits the largest representable duration to approximately 290 years.

which explains why 100000 days (around 274 years) works but 110000 days (301 years) fails.

Personally I prefer the 400 response. but both numbers aren't logically speaking "invalid", so should we simply silently ignore Max-Days bigger than 106752 days so that the uploaded file acts like it has no expired date?

@kugiyasan
Copy link
Contributor Author

Oops, sorry @aspacca, the tests were passing and I merged the main branch, can you re-run the tests and merge please?

@aspacca aspacca merged commit 2959fc2 into dutchcoders:main Oct 25, 2021
aspacca added a commit that referenced this pull request Oct 26, 2021
aspacca added a commit that referenced this pull request Oct 26, 2021
GanZhiXiong added a commit to GanZhiXiong/transfer.sh that referenced this pull request Nov 10, 2021
…ch-1

* 'patch-1' of github.com:GanZhiXiong/transfer.sh:
  issue dutchcoders#420 added MaxDate.IsZero() check (dutchcoders#427)
  fix go1.16, add go1.17
  bump bluemonday
  Revert "issue dutchcoders#420 return 400 response when Max-Days is too big (dutchcoders#422)" (dutchcoders#426)
  issue dutchcoders#420 return 400 response when Max-Days is too big (dutchcoders#422)
  Implement Nix Flake (dutchcoders#424)
  fix missed errors (dutchcoders#417)
  Fix path (dutchcoders#416)
  Edited code of condunct for more information and corrected a grammatical error (dutchcoders#421)
  Update README.md (dutchcoders#415)
  Update README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants