Skip to content

Conversation

@esindril
Copy link
Contributor

The resource string is unquoted when the request is handled but in case of a redirect the resource should be quoted again otherwise the client will be redirected with a different path than the initial one.

@amadio
Copy link
Member

amadio commented Feb 12, 2025

Any chance we could have a test added for this?

@esindril
Copy link
Contributor Author

Sure, I will push something for this.

@esindril esindril marked this pull request as draft February 13, 2025 07:15
The resource string is unquoted when the request is handled but in
case of a redirect the resource should be quoted again otherwise
the client will be redirected with a different path than the initial
one.
@esindril esindril force-pushed the http_quote_resource_rdr branch from f2dc469 to a8eb442 Compare February 13, 2025 07:16
@amadio
Copy link
Member

amadio commented Feb 13, 2025

Look in tests/XRootD/http.sh. It might be just a matter of adding a few commands there.

@esindril esindril marked this pull request as ready for review February 13, 2025 08:29
@esindril
Copy link
Contributor Author

Just a few extra remarks related to this pull request:

  • this fixes my particular problem with handling # in the file names, but in general the quote and unquote methods are not consistent/symmetric.
  • I have modified the quote method to not quote the path separator / as this would screw up all the paths when redirecting. It's not clear to me what was the initial use-case for quoting /.
  • There should be an alignment between what XRootD accepts in terms of valid characters for paths https://xrootd.web.cern.ch/doc/dev6/XRdv520.htm#_Toc173272867 and what (HTTP) URI considers reserved characters https://datatracker.ietf.org/doc/html/rfc3986#page-12
  • When configuring the xroot and http support on different ports the redirection when coming with a HTTP request will alwatys wrongly redirect to the XRD port (maybe there is some hidden option to fix this?!). Therefore, in order to have HTTP support at the cluster level the XROOTD and HTTP protocols need to be served on the same port.

@amadio
Copy link
Member

amadio commented Feb 13, 2025

Something that was added doesn't work as expected: https://my.cdash.org/tests/253509135

@esindril
Copy link
Contributor Author

Yes, curl was missing from the Alpine image.

@esindril esindril force-pushed the http_quote_resource_rdr branch from 7d95fca to 1d33347 Compare February 13, 2025 09:38
that are accessed both via HTTP and XRootD.

Extend the cluster tests configuration by enabling HTTP support and
exercise uploads, downloads also via HTTP protocol for data that is
distributed in the cluster. The main target of the new tests is to
ensure the proper unquoting and quoting functionality in the XrdHttp
layer.
@esindril esindril force-pushed the http_quote_resource_rdr branch from 1d33347 to efc54b7 Compare February 13, 2025 09:47
@amadio amadio added this to the 5.8.0 milestone Feb 13, 2025
@amadio amadio merged commit 19b8e5e into xrootd:master Feb 13, 2025
11 checks passed
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 this pull request may close these issues.

2 participants