summaryrefslogtreecommitdiff
path: root/methods
Commit message (Collapse)AuthorAgeFilesLines
* Rewrite HttpServerState::Die()Julian Andres Klode2020-08-111-28/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* http: Fully flush local file both before/after server readJulian Andres Klode2020-08-113-19/+26
| | | | | | | | | | | 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.
* http: Do not use non-blocking local I/OJulian Andres Klode2020-08-111-10/+0
| | | | This causes some more issues, really.
* http: Restore successful exits from Die()Julian Andres Klode2020-08-111-4/+6
| | | | | We have successfully finished reading data if our buffer is empty, so we don't need to do any further checks.
* Do not retry on failure to fetchJulian Andres Klode2020-08-101-20/+13
| | | | | | | | | | | | | While we fixed the infinite retrying earlier, we still have problems if we retry in the middle of a transfer, we might end up resuming downloads that are already done and read more than we should (removing the IsOpen() check so that it always retries makes test-ubuntu-bug-1098738-apt-get-source-md5sum fail with wrong file sizes). I think the retrying was added to fixup pipelining messups, but we have better solutions now, so let's get rid of it, until we have implemented this properly.
* basehttp: Correctly handle non-transient failure from RunData()Julian Andres Klode2020-08-051-12/+3
| | | | | | | | | | When we failed after a retry, we only communicated failure as transient, but this seems wrong, especially given that the code now always triggers a retry when Die() is called, as Die() closes the server fd. Instead, remove the error handling in that code path, and reuse the existing fatal-ish error code handling path.
* http: Fix infinite loop on read errorsJulian Andres Klode2020-08-051-3/+10
| | | | | | | | | | | | If there was a transient error and the server fd was closed, the code would infinitely retry - it never reached FailCounter >= 2 because it falls through to the end of the loop, which sets FailCounter = 0. Add a continue just like the DNS rotation code has, so that the retry actually fails after 2 attempts. Also rework the error logic to forward the actual error message.
* Merge branch 'pu/http-fixes-2' into 'master'Julian Andres Klode2020-08-041-1/+2
|\ | | | | | | | | Pu/http fixes 2 See merge request apt-team/apt!125
| * http: Always write to the file if there's something to writeJulian Andres Klode2020-08-041-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We only add the file to the select() call if we have data to write to it prior to the select() call. This is problematic: Assuming we enter Go() with no data to write to the file, but we read some from the server as well as an EOF, we end up not writing it to the file because we did not add the file to the select. We can't always add the file to the select(), because it's basically always ready and we don't want to wake up if we don't have anything to read or write. So for a solution, let's just always write data to the file if there's data to write to it. If some gets leftover, or if some was already present when we started Go(), it will still be added to the select() call and unblock it. Closes: #959518
* | Merge branch 'pu/less-slaves' into 'master'Julian Andres Klode2020-08-042-7/+7
|\ \ | |/ |/| | | | | Remove master/slave terminology See merge request apt-team/apt!124
| * gpgv: Rename master to primaryJulian Andres Klode2020-08-041-4/+4
| |
| * CMake: Rename add_slaves() to add_links()Julian Andres Klode2020-07-141-3/+3
| | | | | | | | Sorry!
* | http: Redesign reading of pending dataJulian Andres Klode2020-07-241-10/+8
| | | | | | | | | | | | | | | | | | | | Instead of reading the data early, disable the timeout for the select() call and read the data later. Also, change Read() to call only once to drain the buffer in such instances. We could optimize this to call read() multiple times if there is also pending stuff on the socket, but that it slightly more complex and should not provide any benefits.
* | http: On select timeout, error out directly, do not call Die()Julian Andres Klode2020-07-241-1/+1
| | | | | | | | | | The error handling in Die() that's supposed to add useful error messages is not super useful here.
* | http: Finish copying data from server to file before sending stuff to serverJulian Andres Klode2020-07-241-7/+7
| | | | | | | | | | | | This avoids a case where we read data, then write to the server and only then realize the connection was closed. It is somewhat slower, though.
* | http: Die(): Do not flush the buffer, error out insteadJulian Andres Klode2020-07-241-18/+3
| | | | | | | | | | | | | | By changing the buffer implementation to return true if it read or wrote something, even on EOF, we should not have a need to flush the buffer in Die() anymore - we should only be calling Die() if the buffer is empty now.
* | http: Only return false for EOF if we actually did not read anythingJulian Andres Klode2020-07-241-4/+4
| | | | | | | | | | | | | | | | | | This should avoid the need to Flush the buffer in Die(), because if we read anything, we are returning true, and not entering Die() at that point. Also Write() does not have a concept of EOF, so get rid of code handling that there. Was that copied from Read()?
* | http: Die(): Merge flushing code from Flush()Julian Andres Klode2020-07-241-0/+5
| | | | | | | | | | | | | | Die() needs its own Copy() of Flush() because it needs to return success or failure based on some states, but those are not precisely the same as Flush(), as Flush() will always return false at the end, for example, but we want to fall through to our error handling.
* | http: Always Close() the connection in Die()Julian Andres Klode2020-07-241-2/+2
|/ | | | | If we reached Die() there was an issue with the server connection, so we should always explicitly close it.
* Reorder config check before checking systemd for non-interactive httpDavid Kalnischkies2020-07-021-9/+13
| | | | | | If this option is disabled (which it is by default in Debian), we don't have to make the call and the checks around it. Not that it really matters that much as if it would we would be better checking only once.
* Replace some magic 64*1024 with APT_BUFFER_SIZEJulian Andres Klode2020-06-231-1/+1
|
* ubuntu: http: Add non-interactive to user agent if run by systemdJulian Andres Klode2020-04-092-2/+21
| | | | | | | | | | | | | | | Include that apt is being run from a service in the user agent, so traffic can be analysed for interactive vs non-interactive use, and prioritised accordingly. It looks like this now: User-Agent: Debian APT-HTTP/1.3 (2.0.1) non-interactive A previous version included the full service names, but this raised some privacy concerns. LP: #1825000
* cdrom: Remove old udev dlopen stuffJulian Andres Klode2020-02-261-1/+0
|
* Remove code tagged APT_PKG_590, add some missing includesJulian Andres Klode2020-02-181-0/+1
| | | | | | Remove all code scheduled to be removed after 5.90, and fix files to include files they previously got from hashes.h including more headers.
* seccomp: Allow recvmmsg_time64() and futex_time64()Julian Andres Klode2020-02-101-0/+2
| | | | | Saw those in libseccomp commit, figured should add them too.
* seccomp: Allow time64 variants (>402,<415) of allowed syscallsJulian Andres Klode2020-02-091-0/+6
| | | | | | | | Went through the list of syscalls starting at 403 (and looked one back) up to the next unknown syscalls, and added all those that were present in their non-time64 variant. Closes: #951012
* netrc: Add warning when ignoring entries for unencrypted protocolsJulian Andres Klode2020-01-151-0/+12
| | | | | | | | Commit 93f33052de84e9aeaf19c92291d043dad2665bbd restricted auth.conf entries to only apply to https by default, but this was silent - there was no information why http sources with auth.conf entries suddenly started failing. Add such information, and extend test case to cover it.
* Rename _count() macro to APT_ARRAY_SIZE()Julian Andres Klode2020-01-071-1/+2
|
* Fix some style warnings from cppcheckDavid Kalnischkies2019-11-261-11/+9
| | | | | | | | Unused variable, std::algorithms instead of raw for-loops. There should be no observeable difference in behaviour. Reported-By: cppcheck Gbp-Dch: Ignore
* Fix typos reported by codespell in code commentsDavid Kalnischkies2019-07-102-5/+6
| | | | | | | | Also in old changelogs, but nothing really user visible like error messages or alike so barely noteworthy. Reported-By: codespell Gbp-Dch: Ignore
* Apply various suggestions by cppcheckDavid Kalnischkies2019-07-0810-22/+16
| | | | Reported-By: cppcheck
* RFC1123StrToTime: Accept const std::string& as first argumentJulian Andres Klode2019-06-171-1/+1
| | | | | | We are converting to std::string anyway by passing to istringstream, and this removes the need for .c_str() in callers.
* http: Fix Host header in proxied https connectionsSimon Körner2019-06-111-3/+3
| | | | | | | | | | | | | | Currently CONNECT requests use the name of the proxy as Host value, instead of the origin server's name. According to RFC 2616 "The Host field value MUST represent the naming authority of the origin server or gateway given by the original URL." The current implementation causes problems with some proxy vendors. This commit fixes this. [jak: Adding a test case] See merge request apt-team/apt!66
* Merge tag '1.8.2'Julian Andres Klode2019-06-111-28/+51
|\ | | | | | | apt Debian release 1.8.2
| * methods: https: handle requests for TLS re-handshakeMichael Zhivich2019-05-211-28/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When accessing repository protected by TLS mutual auth, apt may receive a "re-handshake" request from the server, which must be handled in order for download to proceed. This situation arises when the server requests a client certificate based on the resource path provided in the GET request, after the inital handshake in UnwrapTLS() has already occurred, and a secure connection has been established. This issue has been observed with Artifactory-backed Debian repository. To address the issue, split TLS handshake code out into its own method in TlsFd, and call it when GNUTLS_E_REHANDSHAKE error is received. Signed-off-by: Michael Zhivich <mzhivich@akamai.com> (merged from Debian/apt#93) LP: #1829861
* | apt-pkg: URI: Add 'explicit' to single argument constructorJulian Andres Klode2019-04-309-17/+17
| | | | | | | | | | This needs a fair amount of changes elsewhere in the code, hence this is separate from the previous commits.
* | Add 'explicit' to most single argument constructorsJulian Andres Klode2019-04-301-1/+1
| | | | | | | | | | | | | | | | | | | | | | This prevents implicit conversions that we do not want, such as having a FileFd* being converted to a debListParser. Two cases are not yet handled because they require changes in code using them: 1. The classes in hashes.h 2. The URI class - this one is used quite a lot
* | Fix new gcc-9 warning -Wpessimizing-moveDavid Kalnischkies2019-04-161-1/+1
|/ | | | | | | | warning: moving a local object in a return statement prevents copy elision [-Wpessimizing-move] Reported-By: gcc-9 Gbp-Dch: Ignore
* aptmethod.h: Do not have gcc warning about ignoring write() resultJulian Andres Klode2019-02-051-0/+7
| | | | | This is a special case here, a best effort write, so there's no point in having warnings about it for every method.
* Communicate back which key(s) were used for signingDavid Kalnischkies2019-01-221-19/+60
| | | | | | | Telling the acquire system which keys caused the gpgv method to succeed allows us for now just a casual check if the gpgv method really executed catching bugs like CVE-2018-0501, but we will make use of the information for better features in the following commits.
* Refactor internal Signers information storage in gpgvDavid Kalnischkies2019-01-221-73/+67
| | | | | | | | | Having a method take a bunch of string vectors is bad style, so we change this to a wrapping struct and adapt the rest of the code brushing it up slightly in the process, which results even in a slightly "better" debug output, no practical change otherwise. Gbp-Dch: Ignore
* Add support for /etc/apt/auth.conf.d/*.conf (netrcparts)Julian Andres Klode2018-12-041-11/+36
| | | | | | | | | This allows us to install matching auth files for sources.list.d files, for example; very useful. This converts aptmethod's authfd from one FileFd to a vector of pointers to FileFd, as FileFd cannot be copied, and move operators are hard.
* Fix typo reported by codespell in code commentsDavid Kalnischkies2018-11-251-1/+1
| | | | | | | | No user visible change expect for some years old changelog entries, so we don't really need to add a new one for this… Reported-By: codespell Gbp-Dch: Ignore
* Allow setting Referer header for http methodDavid Kalnischkies2018-11-251-0/+4
| | | | | | Not needed for common interactions, but for some download-file interactions it could be useful to set a specific referer as some servers do not serve requested files otherwise.
* Revert "http: Fix handling of server connection closure"Julian Andres Klode2018-11-131-3/+4
| | | | | | | | This reverts commit fb3f36593563d09a8d1727cc7c6deb0b49823ca2. It caused downloads to hang on long-lived connections on certain servers. Gbp-Dch: full
* http: Fix handling of server connection closureJulian Andres Klode2018-11-121-4/+3
| | | | | | | | | | | | | If the server closed the connection while we're reading data, and we end up not having any data left to write; that is, for example, we received 0 bytes, then we did not exit before, as we only returned success if there was data to write. This is wrong: Obviously, if we have reached our limit, we are done anyway. It's a bit unclear if we actually ever reached this part, but it does make some sense wrt the bug below. LP: #1801338
* Merge branch 'feature/subkeys' into 'master'Julian Andres Klode2018-10-141-18/+64
|\ | | | | | | | | Support subkeys and multiple keyrings in Signed-By options See merge request apt-team/apt!27
| * Support multiple keyrings in sources.list Signed-ByDavid Kalnischkies2018-09-111-11/+29
| | | | | | | | | | | | | | A user can specify multiple fingerprints for a while now, so its seems counter-intuitive to support only one keyring, especially if this isn't really checked or enforced and while unlikely mixtures of both should work properly, too, instead of a kinda random behaviour.
| * Support subkeys properly in Signed-By optionsDavid Kalnischkies2018-09-111-8/+36
| | | | | | | | | | | | | | | | If we limit a file to be signed by a certain key it should usually accept also being signed by any of this keys subkeys instead of requiring each subkey to be listed explicitly. If the later is really wanted we support now also the same syntax as gpg does with appending an exclamation mark at the end of the fingerprint to force no mapping.
* | http: Stop pipeline after close only if it was not filled beforeJulian Andres Klode2018-09-182-6/+21
|/ | | | | | | | | | | | | | It is perfectly valid behavior for a server to respond with Connection: close eventually, even when pipelining. Turning off pipelining due to that is wrong. For example, some Ubuntu mirrors close the connection after 101 requests. If I have more packages to install, only the first 101 would benefit from pipelining. This commit introduces a new check to only turn of pipelining for future connections if the pipeline for this connection did not have 3 successful fetches before, that should work quite well to detect broken server/proxy combinations like in bug 832113.
generated by cgit v1.2.3-70-g09d2 (git 2.46.0) at 2024-11-05 03:43:52 +0000