request #13492 Improve TUS implementation to support client unable to use PATCH
    Guilhem Bonnefille (CS) (gbonnefille)
    2019-08-23 08:55
    2019-06-14 22:11
    Improve TUS implementation to support client unable to use PATCH
    While trying to upload file from Jenkins Pipeline, I'm faced to an inextricable issue.
    TUS protocol suggest a PATCH to send content. But this method is not supported by OpenJDK: https://bugs.openjdk.java.net/browse/JDK-7016595
    In such situation, Tus Java implementation switches to POST+X-HTTP-Method-Override=PATCH:

    This behavior is quite standard and documented in TUS: https://tus.io/protocols/resumable-upload.html#x-http-method-override

    But it seems the TUS implementation in Tuleap does not support this feature. The POST method is not routed and the TusServer does not check this situation.
    Other PHP implementation explicitly support and test this approach: https://github.com/ankitpokhrel/tus-php

    As Jenkins is the reference CI system, it would be useful to be compliant with this solution to let Java client working.
    Delivery/File release system
    Thomas Gerbet (tgerbet)


    • User avatar
      • Status changed from Under review to Closed
      • Close date set to 2019-08-23
    • User avatar
      gerrit #15379 integrated into Tuleap
    • User avatar

      We can either create a git repo in the Tuleap project otherwise you can create your own project here on tuleap.net and organise yourself as you wish (on the plus side, your contrib won't be lost amongts all the other git repo of the projet).

    • User avatar
      Thanks for solving this issue.

      I will check what I can release or not. Is there any "contrib" sub-directory or repository to submit contributions related to Tuleap? I know we also have an other piece of code for Jenkins library in order to submit build status associated to a commit.
    • User avatar

      @gbonnefille Thanks for reporting that, could you please share the code snippet you are using in Jenkins ? I'm pretty sure it could be an handy addition to the doc.


    • User avatar
      gerrit #15261 integrated into Tuleap
    • User avatar
      Support of the X-Http-Method-Override for the requests served with the tus protocol is currently being reviewed here: gerrit #15261.

      • Status changed from Verified to Under review
      • Assigned to changed from None to Thomas Gerbet (tgerbet)
    • User avatar
      Sorry, I did not understand.
      Reading "Running a production instance with a large value for the memory_limit directive is IMO not suitable at all for production instances" I understood you want to implement a solution supporting HUGE POST actions.

      In the case of TUS, the post_max_size/upload_max_size directives are not related to the chunk but will be associated to the resulting file?
    • User avatar
      Yes, this exactly what the protocol state and my previous follow up explains why the mutation of the HTTP verb according to the X-HTTP-Method-Override header has not been implemented.
    • User avatar
      Looking deeper in this topic, it seems that the X-HTTP-Method-Override value should be used as the **REAL** method called. The wrapping POST call should be considered as a transport solution only.

      This behavior is clearly observable in the TUS implementations:
      - https://github.com/tus/tusd
      - https://github.com/tus/tus-node-server

      If supporting X-HTTP-Method-Override in a POST, you should not implement a single huge upload but really the full TUS protocol, with PATCH wrapped into POST.
    • User avatar
      As I'm not HTTP expert, I initially imagined that X-HTTP-Method-Override allows to implement a PATCH logic using POST vocabulary. Of course, if you have to implement a true POST method, it would be less usefull, particularly for really large files (I'm trying to upload Docker image dump).

      Are you sure you have to implement a real POST method?
    • User avatar

      Yes the part of the tus protocol about the handling of the X-HTTP-Method-Override header is not respected.
      To be honest this omission was voluntary. Allowing large uploads with POST requests means that we need to have the post_max_size/upload_max_size directives set accordingly and since PHP by default will read the data to populate the $_POST and $_FILES which means having a memory_limit directive sets to a larger value than the post_max_size directive. Running a production instance with a large value for the memory_limit directive is IMO not suitable at all for production instances and we cannot disable the directive enable_post_data_reading because large parts of Tuleap require it.

      This is why the switch with the X-HTTP-Method-Override header is not supported ATM (it's an ugly hack after all). That's said all the routes handling the large file upload are segregated from the others and already have some specific configurations, we might have a way to disable the directive enable_post_data_reading for those routes only.

      • Status changed from New to Verified