eu-digital-identity-wallet / eudi-lib-ios-openid4vci-swift

Implementation of OpenID for Verifiable Credential Issuance protocol (wallet's role) in Swift
Apache License 2.0
6 stars 7 forks source link

[fix] x-www-form-urlencoded body encoding in HTTP form POST #54

Closed srosenda closed 2 weeks ago

srosenda commented 1 month ago

Description of change

Fix the FormPost body encoding when content type is x-www-form-urlencoded. The current implementation does not handle correctly line endings, special characters (e.g. '+') and does not convert spaces to '+' characters.

See https://url.spec.whatwg.org/#application/x-www-form-urlencoded

Additional changes and their rationale

Changes mandatory for implementing the fix:

Other improvements:

Fixes #55.

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Checklist:

srosenda commented 1 month ago

Note that there is almost a verbatim copy in the eudi-lib-ios-siop-openid4vp-swift library of the same FormPost component and its dependencies, including Request protocol that are fixed in this PR, see https://github.com/eu-digital-identity-wallet/eudi-lib-ios-siop-openid4vp-swift/blob/427be9f7e6ac298f9f807a145c479519a8657274/Sources/Main/Services/VerifierFormPost.swift. Should this kind of common utility classes be extracted to a common library?

dtsiflit commented 1 month ago

Great work here @srosenda , we wanted to revisit FormPost ourselves for ages :) Please bear with us because it will be several days before we look into this PR. Thanks!

srosenda commented 1 month ago

Rebased with main. Planning to add some tests to validate the non-trivial x-www-form-urlencoded encoding.

srosenda commented 1 month ago

Test for FormPost added in 1769c56 and the implementation changed to follow the whatwg/url specification more closely.

dtsiflit commented 1 month ago

Note that there is almost a verbatim copy in the eudi-lib-ios-siop-openid4vp-swift library of the same FormPost component and its dependencies, including Request protocol that are fixed in this PR, see https://github.com/eu-digital-identity-wallet/eudi-lib-ios-siop-openid4vp-swift/blob/427be9f7e6ac298f9f807a145c479519a8657274/Sources/Main/Services/VerifierFormPost.swift. Should this kind of common utility classes be extracted to a common library?

Thanks for this feedback @srosenda, unfortunately no plans for a common library at the moment.

srosenda commented 2 weeks ago

Thanks a lot for approving and merging this PR @dtsiflit! I was able to switch our iOS wallet implementation using this library (still at PoC state) to use the upstream repository instead of a patch branch in a fork repository.