Skip to content

Drop HSTS Preloading - #1110

Merged
lovelydinosaur merged 4 commits into
masterfrom
drop-hsts
Aug 5, 2020
Merged

Drop HSTS Preloading#1110
lovelydinosaur merged 4 commits into
masterfrom
drop-hsts

Conversation

@florimondmanca

@florimondmancaflorimondmanca commented Aug 1, 2020

Copy link
Copy Markdown
Contributor

Maybe? See the rationale in #1102

Fixes #1102, closes #896

Essentially:

>>>importhttpx>>>r=httpx.get('http://www.paypal.com') # -> Now kept over HTTP rather than forced to HTTPS.

I suppose if we want to move forward with this we'd want it in 0.14, rather than 1.0, since it might be a small breaking change?

Since this was an always-on feature not controlled by any options, I can't think of a smooth deprecation path, but any ideas welcome!

@florimondmancaflorimondmanca added the api change PRs that contain breaking public API changes label Aug 1, 2020
@florimondmancaflorimondmanca added this to the v0.14 milestone Aug 1, 2020
@lovelydinosaurlovelydinosaur mentioned this pull request Aug 2, 2020
@StephenBrown2

Copy link
Copy Markdown
Contributor

Perhaps hstspreloading could be kept, but not enforced by default, and a TRACE or DEBUG message emitted when a host is found on the list and doesn't match the preferred scheme?

@lovelydinosaur

Copy link
Copy Markdown
Contributor

So, it's a wooly issue, but I think yes let's get this in and treat HSTS as appropriate as a browser feature, but not necessarily desirable in client library.

@lovelydinosaur
lovelydinosaur merged commit 78cf16a into masterAug 5, 2020
@lovelydinosaur
lovelydinosaur deleted the drop-hsts branch August 5, 2020 12:05
@StephenBrown2

Copy link
Copy Markdown
Contributor

I would hate for the great work in the hstspreload module to go unused. Would it be good to include a brief mention in the docs for those that do want that functionality?

@florimondmanca

florimondmanca commented Aug 5, 2020

Copy link
Copy Markdown
ContributorAuthor

I'm trying to think of how people could enable this without us having to support it in core with a flag…

I don't think a custom transport would be the way to go, since transports don't deal with client logic like "optionally modify the request URL".

Probably more like a client subclass, then…?

importhttpximporthstspreloadclassHSTSPreloadMixin:
defbuild_request(self, *args, **kwargs):
request=super().build_request(*args, **kwargs)
url=request.urlif (
url.scheme=="http"andhstspreload.in_hsts_preload(url.host)
andlen(url.host.split(".")) >1
):
port=Noneifurl.port==80elseurl.portrequest.url=url.copy_with(scheme="https", port=port)
returnrequestclassAsyncClient(HSTSPreloadMixin, httpx.AsyncClient):
passclassClient(HSTSPreloadMixin, httpx.Client):
pass

Incidentally I think this could also fit in a "middleware" kind of concept (#345, also mentioned in #984), but that's definitely not something we'll have 1.0.

@StephenBrown2

StephenBrown2 commented Aug 5, 2020

Copy link
Copy Markdown
Contributor

Possibly, but I was thinking of and even simpler approach/demonstration:

util.py

fromtypingimportUnionfromhttpximportURLfromhstspreloadimportin_hsts_preloaddefcheck_hsts(url: Union[str, URL]):
ifisinstance(url, str):
url=URL(url)
ifin_hsts_preload(url.host):
returnurl.copy_with(scheme="https")
returnurl

...
main.py

importhttpxfromutilimportcheck_hstshttpx.get(check_hsts(the_url))

or with base_url:

withhttpx.Client(base_url=check_hsts(the_url)) asclient:
client.get(path)

This was referenced Aug 5, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api changePRs that contain breaking public API changes

Development

Successfully merging this pull request may close these issues.

Consider dropping HSTS preloading Document HSTS is still honored if allow_redirects=False

3 participants

@florimondmanca@StephenBrown2@lovelydinosaur