Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Fix relative paths #26

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

utopiabound
Copy link

If baseurl with an existing path on App, don't overwrite it, just append to it.

Problem is that if baseurl is "https://hostname/acme/" the response to https://hostname/acme/order would return an Order with in part:

"authorizations": [
    "https://hostname/authz/:ID"
  ],

when it should be "https://hostname/acme/authz/..."

@utopiabound utopiabound marked this pull request as draft March 23, 2023 14:54
If baseurl with an existing path on App, don't overwrite it, just append to it.

Problem is that if baseurl is "https://hostname/acme/" the response to https://hostname/acme/order
would return an Order with in part:
```
"authorizations": [
    "https://hostname/authz/:ID"
  ],
```
when it should be "https://hostname/acme/authz/..."

Signed-off-by: Nathaniel Clark <[email protected]>
@utopiabound utopiabound marked this pull request as ready for review March 24, 2023 19:47
Signed-off-by: Nathaniel Clark <[email protected]>
@erikh
Copy link
Contributor

erikh commented Apr 24, 2023

Sorry just got to looking at this. I would triple-check this runs against the test suite, as I strongly suspect one of these approaches is wrong. I would also double check this manually with certbot and "weird" situations by hand to ensure this is a longer-term change.

Not saying you're wrong, but that this has potential to break the whole library, and that appropriate precautions are taken.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants