Debians commandline package manager https://git.kalnischkies.de/apt/atom?h=2.1.16 2020-12-22T23:54:14Z 2020-12-22T23:54:14Z Faidon Liambotis paravoid@debian.org 2020-12-22T23:54:14Z urn:sha1:1663774bf309fbd196fd2b9c5c2afdd7a25fd288 The "last connection" cache is currently being stored and looked up on the combination of (LastHost, LastPort). However, these are not what the arguments to getaddrinfo() were on the first try: the call is to getaddrinfo(Host, ServiceNameOrPort, ...), i.e. with the port *or if 0, the service name* (e.g. http). Effectively this means that the connection cache lookup for: https://example.org/... i.e. Host = example.org, Port = 0, Service = http would end up matching the "last" connection of (if existed): https://example.org/... i.e. Host = example.org, Port = 0, Service = https ...and thus performing a TLS request over an (unrelated) port 80 connection. Therefore, an HTTP request, followed up by an (unrelated) HTTPS request to the same server, would always fail. Address this by using as the cache key the ServiceNameOrPort, rather than Port. 2020-12-22T23:51:50Z Faidon Liambotis paravoid@debian.org 2020-12-22T23:51:50Z urn:sha1:8d4b3a4fcead0ca534b5d1c5a99ae2a4c95eee21 Convert the fixed-size (300) char array "ServStr" to a std::string, and simplify the code by removing snprintfs in the process. While at it, rename to the more aptly named "ServiceNameOrPort" and update the comment to reflect what this variable is meant to be. 2020-12-22T23:23:22Z Faidon Liambotis paravoid@debian.org 2020-12-22T23:23:22Z urn:sha1:570861fc55ba38c1092fac1d555111bab4577b49 ServerState->Comp() is used by the HTTP methods main loop to check whether a connection can be reused, or whether a new one is needed. Unfortunately, the currently implementation only compares the Host and Port between the ServerState's internal URI, with a new URI. However these are URIs, and therefore Port is 0 when a URI port is not specificied, i.e. in the most common configurations. As a result, a ServerState for http://example.org/... will be reused for URIs of the form https://example.org/..., as both Host (example.org) and Port (0) match. In turn this means that GET requests will happen over port 80, in cleartext, even for those https URLs(!). URI Acquires for an http URI and subsequently for an https one, in the same aptmethod session, do not typically happen with apt as the frontend, as apt opens a new pipe with the "https" aptmethod binary (nowadays a symlink to http), which is why this hasn't been a problem in practice and has eluded detection so far. It does happen in the wild with other frontends (e.g. reprepro), plus is legitimately an odd and surprising behavior on apt's end. Therefore add a comparison for the URI's "Access" (= the scheme) in addition to Host and Port, to ensure that we're not reusing the same state for multiple different schemes. 2020-12-18T18:31:19Z David Kalnischkies david@kalnischkies.de 2020-07-09T22:02:25Z urn:sha1:97d6c3b2d05fe0d965657197adf56cc78f9edf81 Every method opts in to getting the encoded URI passed along while keeping compat in case we are operated by an older acquire system. Effectively this is just a change for the http-based methods as the others just decode the URI as they work with files directly. 2020-11-07T21:52:20Z David Kalnischkies david@kalnischkies.de 2020-11-07T21:52:20Z urn:sha1:e5bb443cf58cec23503ad0deeeb06a080053da8a 2020-11-07T20:48:21Z David Kalnischkies david@kalnischkies.de 2020-11-07T20:39:00Z urn:sha1:418f9272606857e312f485778a1ef1b263236463 The acquire system mode does this for a long time already and as it is easy to implement and handy for manual testing as well we can support it in the other modes, too. 2020-11-07T20:48:21Z David Kalnischkies david@kalnischkies.de 2020-11-07T20:23:57Z urn:sha1:9e1398b164f55238990907f63dfdef60588d9b24 Merging patches is a bit of non-trivial code we have for client-side work, but as we support also server-side merging we can export this functionality so that server software can reuse it. Note that this just cleans up and makes rred behave a bit more like all our other binaries by supporting setting configuration at runtime and supporting --help and --version. If you can make due without this, the now advertised functionality is provided already in earlier versions. 2020-08-11T11:42:41Z Julian Andres Klode julian.klode@canonical.com 2020-08-11T11:09:14Z urn:sha1:8b35e2a3dd7b863639a8909fa2361ed4fd217bc3 The old code was fairly confusing, and contradictory. Notably, the second `if` also only applied to the Data state, whereas we already terminated the Data state earlier. This was bad. The else fallback applied in three cases: (1) We reached our limit (2) We are Persistent (3) We are headers Now, it always failed as a transient error if it had nothing left in the buffer. BUT: Nothing left in the buffer is the correct thing to happen if we were fetching content. Checking all combinations for the flags, we can compare the results of Die() between 2.1.7 - the last "known-acceptable-ish" version and this version: 2.1.7 this Data !Persist !Space !Limit OK (A) OK Data !Persist !Space Limit OK (A) OK Data !Persist Space !Limit OK (C) OK Data !Persist Space Limit OK OK Data Persist !Space !Limit ERR ERR * Data Persist !Space Limit OK (B) OK Data Persist Space !Limit ERR ERR Data Persist Space Limit OK OK => Data connections are OK if they have not reached their limit, or are persistent (in which case they'll probably be chunked) Header !Persist !Space !Limit ERR ERR Header !Persist !Space Limit ERR ERR Header !Persist Space !Limit OK OK Header !Persist Space Limit OK OK Header Persist !Space !Limit ERR ERR Header Persist !Space Limit ERR ERR Header Persist Space !Limit OK OK Header Persist Space Limit OK OK => Common scheme here is that header connections are fine if they have read something into the input buffer (Space). The rest does not matter. (A) Non-persistent connections with !space always enter the else clause, hence success (B) no Space means we enter the if/else, we go with else because IsLimit(), and we succeed because we don't have space (C) Having space we do enter the while (WriteSpace()) loop, but we never reach IsLimit(), hence we fall through. Given that our connection is not persistent, we fall through to the else case, and there we win because we have data left to write. 2020-08-11T11:09:04Z Julian Andres Klode julian.klode@canonical.com 2020-08-11T08:55:09Z urn:sha1:73780d7f664a4ea1da55527d726b4c9c7753f1fb We do not want to end up in a code path while reading content from the server where we have local data left to write, which can happen if a previous read included both headers and content. Restructure Flush() to accept a new argument to allow incomplete flushs (which do not match our limit), so that it can flush as far as possible, and modify Go() and use that before and after reading from the server. 2020-08-11T11:09:04Z Julian Andres Klode julian.klode@canonical.com 2020-08-11T09:42:15Z urn:sha1:13ab2317451931f055855f1aeaec6c8b28b14ce2 This causes some more issues, really.