Tightening public API on models

[tomchristie]

In our lead up to 0.14 we went though the public API of Client in a huge amount of detail to make sure we had everything exactly right, and were only exposing exactly what we needed to the public API.

Eg see issues #997, #1065, #1066

To a lesser extent we still need to make sure to do the same on everything in _models.py before we can hit 1.0.

Here are a few items from having taken a glance over...

• Request.prepare should be private.
• Response should not expose a public .decoder property. Particularly given that we don't expose the Decoder interface anywhere. Let's replace it with a private ._get_content_decoder() method.
• Review Headers and multi-header handling.
• URL.path should be unquoted. Review str vs. bytes and quoting throughout.

[cdeler]

@tomchristie

By

URL.path should be unquoted. Review str vs. bytes and quoting throughout.

did you mean urllib.parse.unquote /urllib.parse.unquote_plus

If so, what about query and full_path?

httpx/httpx/_models.py

Lines 123 to 136 in 4bd08be

	@property
	def path(self) -> str:
	return self._uri_reference.path or "/"
	@property
	def query(self) -> str:
	return self._uri_reference.query or ""
	@property
	def full_path(self) -> str:
	path = self.path
	if self.query:
	path += "?" + self.query
	return path

[tomchristie]

What we really want here is...

• .path - str URL escaped.
• .query - bytes No escaping applied.
• .raw_path - bytes No escaping applied. Includes both path and query portion.

And also related...

• .userinfo - bytes No escaping applied.
• .username- str URL escaped. (We've already got this)
• .password- str URL escaped. (We've already got this)

And...

• QueryParams should accept bytes alongside the existing str, so that both this... QueryParams("a=123&b=456") and QueryParams(b"a=123&b=456") are valid.

We can handle that by adding bytes to the QueryParamsTypes in _types.py, and by supporting bytes in the QueryParams instantiation in _models.py...

        if value is None or isinstance(value, (str, bytes)):
            value = value.decode("ascii") if isinstance(value, bytes) else value
            items = parse_qsl(value)

There's probably also some thinking about if we'd like .host to expose the IDNA decoded host, and add a .raw_host for the byte-wise IDNA encoded hostname.

That'll give us a more precise API where bytes are used for raw unescaped values, and str is for nicely escaped representations.

[cdeler]

@tomchristie

I'm a bit confused with one small detail, I'm sorry for the question, but

.query - bytes No escaping applied.

for now it's a str, moreover it's stored as a str

In the example your provided with, value is also a str:

        if value is None or isinstance(value, (str, bytes)):
            value = value.decode("ascii") if isinstance(value, bytes) else value
            items = parse_qsl(value)

The question is: may be it's more convenient to treat URL.query as a str with escaping, but give an alternative, e.g. raw_query?

----
Update
And again, sorry for the question, but given the URL: http://example.com/photo.jpg?version=1,
URL.path is "/photo.jpg" -- str with unquote applied
URL.raw_path is b"/photo.jpg?version=1" -- bytes without unquote
URL.full_path is "/photo.jpg?version=1" -- str ??? I'm not sure about urllib.parse.unquote

From my point of view it might mislead someone (but it's ok if it's a part of the internal API) -- how do path, raw_path and full_path correlates?

[tomchristie]

To clear up one part of this... we should rename .full_path to .raw_path (which should return bytes)

[tomchristie]

Note to self wrt. comma delimited headers: https://github.com/bnoordhuis/mozilla-central/blob/master/netwerk/protocol/http/nsHttpHeaderArray.h#L185-L187