search

tus / tus-java-client

The tus client for Java.
https://tus.io
MIT License
213 stars 88 forks source link

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

Closed barkeldiho closed 1 year ago

barkeldiho commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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: https://github.com/tus/tus-java-client/issues/78