search

tus / tusd

Reference server implementation in Go of tus: the open protocol for resumable file uploads
https://tus.github.io/tusd
MIT License
3.08k stars 483 forks source link

Incorrect handling of "Upload too big" response to pre_create hook #1195

Closed YesThatGy closed 1 month ago

YesThatGy commented 1 month ago

Describe the bug When a pre_create hook is rejected due to being too large, tusd responds with a generic HTTP 500 causing my tus client to report to the user that the server is broken, rather than be able to discern that the problem really exists with the client's file being too large.

To Reproduce Steps to reproduce the behavior:

  1. Configure tusd to use hooks. My incantation looks like:

HOST="https://mysite.com/api/hooks"; $PATH/tusd/tusd_linux_amd64/tusd \ -hooks-http $HOST \ -disable-download \ --upload-dir "$PATH/data/"

  1. Configure $HOST to respond with either as follows if the size of the file being upload exceeds a specified limit. (EG: 100 MB)

HTTP_CODE: 413 RESPONSE_BODY: {"HTTPResponse":{"StatusCode":413,"Body":"{\"message\" => \"The upload is too big\"}","Header":{"Content-Type":"application\/json"}},"RejectUpload":"true"}

HTTP_CODE: 507 RESPONSE BODY: {"HTTPResponse":{"StatusCode":507,"Body":"{\"message\" => \"The upload is too big\"}","Header":{"Content-Type":"application\/json"}},"RejectUpload":"true"}

  1. Look at the output of tusd:

2024/09/29 15:31:09.472325 level=INFO event=RequestIncoming method=POST path="" requestId="" 2024/09/29 15:31:09.474989 level=DEBUG event=HookInvocationStart type=pre-create id="" 2024/09/29 15:31:13.905467 level=ERROR event=HookInvocationError type=pre-create id="" error="unexpected response code from hook endpoint (507): {\"HTTPResponse\":{\"StatusCode\":507,\"Body\":\"{\\"message\\" => \\"The upload is too big\\"}\",\"Header\":{\"Content-Type\":\"application\/json\"}},\"RejectUpload\":\"true\"}" 2024/09/29 15:31:13.905883 level=ERROR event=InternalServerError method=POST path="" requestId="" message="unexpected response code from hook endpoint (507): {\"HTTPResponse\":{\"StatusCode\":507,\"Body\":\"{\\"message\\" => \\"The upload is too big\\"}\",\"Header\":{\"Content-Type\":\"application\/json\"}},\"RejectUpload\":\"true\"}" 2024/09/29 15:31:13.906239 level=INFO event=ResponseOutgoing method=POST path="" requestId="" status=500 body="ERR_INTERNAL_SERVER_ERROR: unexpected response code from hook endpoint (507): {\"HTTPResponse\":{\"StatusCode\":507,\"Body\":\"{\\"message\\" => \\"The upload is too big\\"}\",\"Header\":{\"Content-Type\":\"application\/json\"}},\"RejectUpload\":\"true\"}\n"

  1. See error returned to client. In my case it's: processLoop Exception: Exception: ProtocolException: (500) (which matches the last line of output from tusd)

Expected behavior Tusd client should have access to HTTP codes with more information than just 500. If it's a size issue (as what I found in my case debugging a generic "Unexpected Error") then 413 or perhaps 507 would be appropriate.

Setup details Please provide following details, if applicable to your situation:

Acconut commented 1 month ago

A HTTP hook itself must always respond with a 2XX status code, even if the hook instructs tusd to respond with a non-2XX status code to the client. Any other status code is interpreted as a failure to execute the hook itself. Please read https://tus.github.io/tusd/advanced-topics/hooks/#responses for more detail on the responses for HTTP hooks.

YesThatGy commented 1 month ago

Interesting, and that makes sense. I also ran into an issue with

"RejectUpload":"true"

Quoting the "true" cause a parse error, and

and

{\"message\" => \"The upload is too big\"}

should have read

{\"message\" : \"The upload is too big\"}

YesThatGy commented 1 month ago

This issue can be closed.