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

Change in upload link handling. #4441

Closed
yuyichao opened this issue Feb 14, 2020 · 11 comments · May be fixed by haiwen/seafile-server#321
Closed

Change in upload link handling. #4441

yuyichao opened this issue Feb 14, 2020 · 11 comments · May be fixed by haiwen/seafile-server#321

Comments

@yuyichao
Copy link

yuyichao commented Feb 14, 2020

This is a change in the upload API that happens somewhere between 6.3 and 7.1. I was able to use repos/<repo-id>/upload-link/ to get an upload URL and then use the parent_dir=<...> to specify the directoy I want to upload the file into.

Now in 7.1.1, if I don't give the p= parameter for thhe upload-link request, I get {"error": "Permission denied."} as return value (with status code 200 btw). If I only use p= in thte first request but not the parent_dir= in the second request, I get {"error": "Invalid URL. "} (with a new line before the closing quote, which might be invalid JSON, also 200 status code). I had to provide he directory in both requests. This is fine to work with but is a bit strange. The error return is also a bit strange and give no useful information.....

@yuyichao
Copy link
Author

I have a strong suspicion that this is caused by haiwen/seafile-server@9f0083f#diff-92229f8110a875425cedcc84d4d8ba65R504 .

@yuyichao
Copy link
Author

yuyichao commented Feb 14, 2020

Actually, it's probably haiwen/seafile-server#286 instead (edit: the commit above is from this PR but is not the one responsible.)

yuyichao added a commit to yuyichao/seafile-server that referenced this issue Feb 14, 2020
Currently the behavior is such that the directory specified in the
two requests have to match, which makes the argument for the second
request completely useless. If this were to be the desired behavior
then the `parent_dir` parameter should just be ignored from the second
request completely. Alternatively, this commit allows upload to a subdirectory
of the `p` parameter from the first request again.
This matches the old behavior better without allowing uploading to
an arbitrary directory.

If the current behavior is also desired for some use cases,
it should be added as new request or
with a new parameter on the first request.

Fix regression from haiwen#286.

Fix haiwen/seahub#4441
@imwhatiam
Copy link
Member

Hello.

Now in 7.1.1, if I don't give the p= parameter for thhe upload-link request, I get {"error": "Permission denied."} as return value (with status code 200 btw).

I just tested this situation, it returned a url which is supposed to, not {"error": "Permission denied."}.

Can you generate a test account for us to test in your Seafile service ?

@imwhatiam
Copy link
Member

imwhatiam commented Apr 13, 2020

If I only use p= in thte first request but not the parent_dir= in the second request, I get {"error": "Invalid URL. "}

We will update the parameter check logic for the first request (repos/<repo-id>/upload-link/).

If only use p=, Seafile will return an error.

@yuyichao
Copy link
Author

Can you generate a test account for us to test in your Seafile service ?

Not really since I've patched this up locally......... Using the problem mentioned above.......

@yuyichao
Copy link
Author

Now in 7.1.1, if I don't give the p= parameter for thhe upload-link request, I get {"error": "Permission denied."} as return value (with status code 200 btw).

I just tested this situation, it returned a url which is supposed to, not {"error": "Permission denied."}.

BTW, what I meant by the "upload-link" request is the request to the URL I get back from the first request (upload-link) that is used to actually upload the file.

@freeplant
Copy link
Member

Fixed in #4528

@yuyichao
Copy link
Author

AFAICT that doesn't fix the problem I'm reporting.

@freeplant
Copy link
Member

In version 7.1, the parent_dir parameter has to be set both in getting the link and actually uploading file to the link. This is for security reason.

1 similar comment
@freeplant
Copy link
Member

In version 7.1, the parent_dir parameter has to be set both in getting the link and actually uploading file to the link. This is for security reason.

@yuyichao
Copy link
Author

Right, and that is exactly what I argued as not making sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants