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

No error returned when Max-Days is invalid #420

Closed
anihm136 opened this issue Oct 12, 2021 · 7 comments
Closed

No error returned when Max-Days is invalid #420

anihm136 opened this issue Oct 12, 2021 · 7 comments

Comments

@anihm136
Copy link
Contributor

When a file is uploaded with a very high value set for Max-Days header, the server returns a URL. However, the URL leads to a 404 page

Expected behavior:
The server should return an appropriate 400 response when an invalid value is set for the header

@kugiyasan
Copy link
Contributor

Hey can I take this issue?

kugiyasan added a commit to kugiyasan/transfer.sh that referenced this issue Oct 16, 2021
@aspacca
Copy link
Collaborator

aspacca commented Oct 16, 2021

@anihm136 how much high the value must be set?

kugiyasan added a commit to kugiyasan/transfer.sh that referenced this issue Oct 16, 2021
kugiyasan added a commit to kugiyasan/transfer.sh that referenced this issue Oct 16, 2021
@anihm136
Copy link
Contributor Author

Maybe overkill, but I did a binary search and it appears to fail at 8217934 days

aspacca pushed a commit that referenced this issue Oct 25, 2021
* issue #420 return 400 response when Max-Days is too big

Co-authored-by: kugiyasan <kugiyasan@users.noreply.github.com>
@jimmybrancaccio
Copy link

jimmybrancaccio commented Oct 26, 2021

I'm curious if this broke uploading as I am getting a 400 error now on my self-hosted install and when attempting to upload a file via the command line. On the command line I see:

curl --upload-file ./hello.txt http://172.18.0.77:8080/hello.txt
Invalid MaxDate, make sure Max-Days is smaller than 290 years

I didn't provide the header and I assume it shouldn't be a requirement?

Edit:

I wonder if in server/handlers.go it should be something like:

} else if (metadata.MaxDate) && (time.Now().After(metadata.MaxDate)) {

I don't know anything about Go, but I think you may understand my point? Basically it checks to see if metadata.MaxDate has a value and performs the time.Now().After(metadata.MaxDate) condition check.

aspacca added a commit that referenced this issue Oct 26, 2021
aspacca added a commit that referenced this issue Oct 26, 2021
@aspacca
Copy link
Collaborator

aspacca commented Oct 26, 2021

Indeed it seems so @jimmybrancaccio, glad you spotted this so soon.
I've reverted the PR

@kugiyasan it should be something like this, can you create a new PR?

} else if (!metadata.MaxDate.IsZero() && time.Now().After(metadata.MaxDate)) {

thanks

kugiyasan added a commit to kugiyasan/transfer.sh that referenced this issue Oct 27, 2021
@kugiyasan
Copy link
Contributor

Oops sorry! I made the PR asap, @aspacca

aspacca pushed a commit that referenced this issue Oct 30, 2021
* issue #420 return 400 response when Max-Days is too big

* issue #420 moved the Max-Days check before saving the metadata

* issue #420 added a logging message when Max-Days is invalid

* issue #420 added MaxDate.IsZero() check

Co-authored-by: kugiyasan <kugiyasan@users.noreply.github.com>
@jimmybrancaccio
Copy link

Looks good now @aspacca and @kugiyasan Thank you both! ❤️

@aspacca aspacca closed this as completed Nov 8, 2021
GanZhiXiong added a commit to GanZhiXiong/transfer.sh that referenced this issue 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

No branches or pull requests

4 participants