QuakeWang opened a new issue, #778:
URL: https://github.com/apache/opendal-reqsign/issues/778

   ## Description
   
   `reqsign-core::SigningRequest` currently parses the request URI query with 
`form_urlencoded::parse` in `SigningRequest::build`, stores decoded query 
pairs, and rebuilds the URI query from those pairs in `SigningRequest::apply`.
   
   This can change the wire representation of query parameters that already 
existed before signing.
   
   For example, an existing query like:
   
   ```text
   versionId=a%2Bb%3Dc%2525%26e
   ```
   
   can be rebuilt into a raw query like:
   
   ```text
   versionId=a+b=c%25&e
   ```
   
   The rebuilt URI no longer preserves the original encoded value. In 
particular, the encoded `&` inside the value can become a query separator, and 
`+` can also become ambiguous for form-style query parsers.
   
   This matters for downstream users such as OpenDAL. In apache/opendal#7888, 
COS requests need to keep an encoded `versionId` query parameter after signing.
   
   Fixing this in OpenDAL requires service-local query restoration logic, but 
the root issue appears to be that `reqsign-core` does not preserve the raw 
query representation across `SigningRequest::build` and `SigningRequest::apply`.
   
   Review context: 
[https://github.com/apache/opendal/pull/7888#pullrequestreview-4663667735](https://github.com/apache/opendal/pull/7888#pullrequestreview-4663667735)
   
   ## Expected Behavior
   
   Signing should not rewrite query parameters that already existed in the 
request URI.
   
   Signers may still parse and canonicalize query parameters for signature 
calculation. Query-signing implementations may also append authentication query 
parameters. However, existing raw query parameters should keep their original 
wire representation unless a signer explicitly updates them.
   
   ## Scope
   
   This issue is about `reqsign-core::SigningRequest`, not a COS-specific 
workaround in OpenDAL.
   
   At least `reqsign-tencent-cos` is affected because it builds on top of 
`SigningRequest`. Other signers using `SigningRequest` may need to be checked 
as well.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to