Skip to content

Join subdir to base pj URL to preserve path#448

Merged
DanGould merged 1 commit intopayjoin:masterfrom
DanGould:dont-set-path
Dec 29, 2024
Merged

Join subdir to base pj URL to preserve path#448
DanGould merged 1 commit intopayjoin:masterfrom
DanGould:dont-set-path

Conversation

@DanGould
Copy link
Contributor

Fix #416

Without this change a base URL subpath e.g. https://payjo.in/dir-path/ would be overwritten when V2GetContext.extract_req was called and incorrectly produce a request to https://payjo.in/subdir instead of https://payjo.in/dir-path/subdir.

--

This was smoke test by setting let base_url let base_url = self.endpoint.clone().join("gg/") and ensuring thelet url = base_url.join(&subdir.to_string()) .map_err(InternalCreateRequestError::Url)?; preserved the path and resulted in "https://example.com/gg/"

@coveralls
Copy link
Collaborator

coveralls commented Dec 29, 2024

Pull Request Test Coverage Report for Build 12537468509

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 61.852%

Totals Coverage Status
Change from base Build 12528252226: 0.04%
Covered Lines: 2899
Relevant Lines: 4687

💛 - Coveralls

Fix payjoin#416

Without this change a base URL subpath e.g. https://payjo.in/dir-path/
would be overwritten when V2GetContext.extract_req was called and
incorrectly produce a request to https://payjo.in/subdir instead of
https://payjo.in/dir-path/subdir.
Copy link
Collaborator

@nothingmuch nothingmuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK re code changes.

In regards verifying fully resolves the issue, did you do the hacky manual testing we discussed of the directory to ensure end to end flow works, or verify that all instances of setting subdirectory ID consistently use join now?

@DanGould DanGould merged commit e54e51f into payjoin:master Dec 29, 2024
6 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.

Verify v2 client requesting directory hosted with intermediate path elements sets subdirectory ID correctly

3 participants