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

Document the necessity to specifically configure X-HTTP-Method-Override=PATCH #90

Closed
barkeldiho opened this issue Apr 1, 2023 · 4 comments

Comments

@barkeldiho
Copy link

barkeldiho commented Apr 1, 2023

I tried to implement this tus-client for testing purposes of a Tus-compliant API in my Spring Boot application and had the following issue:

Since the TusUploader is using the HttpUrlConnection class to create a HTTP connection it suffers from a wont-fix bug in the OpenJDK (Ref.: https://stackoverflow.com/a/25163132). Consequently, the implementation sends a POST along with "X-HTTP-Method-Override" -> "PATCH" when it is running in JREs that do not support PATCH in that particular class.

Indeed the header "emulates" a PATCH request to be compliant with the TUS protocol specifications, but this needs to be specifically implemented on the server-side. E.g. in my case:
@RequestMapping(method = RequestMethod.POST, headers = {"X-HTTP-Method-Override=PATCH"}, value = "/{id}")

This happened to me on Eclipse Termurin 17.0.5 JDK and is probably a wide-spread problem.
My only recommendation would be to specifically mention this in your documentation as this will reduce implementation overhead. And most importantly: It means that this client is not 100% Tus compliant.

Best

@Acconut
Copy link
Member

Acconut commented Apr 3, 2023

Thank you for bringing this up. I thought that this affected only JDKs which do not support sending PATCH requests, but that there are JDKs which are capable of doing so. Therefore, X-HTTP-Method-Override support should not always be necessary.

We hope to alleviate this issue all together by moving to a more modern HTTP client in the near future anyways.

@barkeldiho
Copy link
Author

barkeldiho commented Apr 4, 2023

Dear Acconut,

thanks for your kind reply. I agree that moving to another HTTP client will solve this issue.
Keep up the good work!

Best

@mdzhigarov
Copy link

Are there any plans to migrate to the new java HttpClient that was introduced with jdk 11? This is a mandatory requirement for making the tus-java-client ready for JDK 17.

@Acconut
Copy link
Member

Acconut commented Oct 19, 2023

@mdzhigarov The new HttpClient looks interesting. I will see if it is suitable for tus-java-client. We plan to upgrade to a more modern HTTP stack for the next bigger release, so this is on our radar: #78

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

3 participants