Fix issues with HTTP redirects and file upload #514
Conversation
The status code 308 (Permanent Redirect) is defined in RFC 7538 and used e.g. by Traefik for redirection. The used version of commons-httpclient doesn't yet include a constant for that status code so use the integer directly. Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
In certain cases when we try to upload a file the server can respond back with a redirect and close the connection before we're done writing content to the server which results in a 'java.net.SocketException: Broken pipe' exception. We can solve that by asking the server first whether we can actually write data (the rest is handled by the httpclient library). See also: http://httpcomponents.10934.n7.nabble.com/Broken-pipe-Write-failed-when-making-Unauthorized-request-tp34235p34245.html Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
Previously redirects with new webdav paths resulted in an IndexOutOfBoundsException as String.substring() was called with a -1 index. Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
Lint
SpotBugs (new)
SpotBugs (master)
|
Codecov Report
@@ Coverage Diff @@
## master #514 +/- ##
==========================================
+ Coverage 39.76% 39.82% +0.06%
==========================================
Files 137 137
Lines 5970 5978 +8
Branches 784 789 +5
==========================================
+ Hits 2374 2381 +7
- Misses 3235 3238 +3
+ Partials 361 359 -2
|
|
Sorry for coming back this late… Have you changed your system after installing the app? |
|
Yes, as mentioned in the original message, the permanent redirect was added after the initial login leading to the issues I was seeing. |
|
Hm. But this then means, that every request is done to times: first against old, get redirect, then to new url. Do you have an idea how to inform the client? |
|
Right.. But as non-HTTP 308 redirect statuses already redirect properly I don't think solving this is in the scope of this - and this PR is just adjusting existing redirect code (for the most part). Also I'm too unfamiliar with the Nextcloud App codebase with regard to updating the base url. |
|
I will have a deeper look into this soon. |
Safe to say this was forgotten. I'll leave this open but my setup has changed in the meantime so I'm not directly interested in this anymore. |
Firstly the test setup:
Previously this resulted in the following notification being sent to the user:
With these patches I can successfully upload pictures to my server.
See commit messages for more details.