| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
| |
We use the code in error messages, so at least for that edgecase we
should ensure that it has sensible content. Note that the acquire
system aborts on non-sensible message content in SendMessage, so you
can't really exploit this.
|
| |
|
|
|
|
|
| |
varg API is a nightmare as the symbols seems different on ever other
arch, but more importantly SendMessage does a few checks on the content
of the message and it is all outputted via C++ iostreams and not mixed
in FILE* which is handy for overriding the streams.
|
| |
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
| |
Reported-By: cppcheck
|
| |
|
|
|
|
| |
We are converting to std::string anyway by passing to
istringstream, and this removes the need for .c_str()
in callers.
|
| |
|
|
|
| |
This needs a fair amount of changes elsewhere in the code,
hence this is separate from the previous commits.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
| |
120s is an insanely high default time out, lower it to 30s
to make things a bit nicer.
|
| |
|
|
|
|
| |
This shouldn't make a practical difference for most people, but for edge
cases it avoids DNS lookups and additionally prevents us from perfoming
unneeded SRV requests, too.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
The Fail method for acquire methods has a boolean parameter indicating
the transient-nature of a reported error. The problem with this is that
Fail is called very late at a point where it is no longer easily
identifiable if an error is indeed transient or not, so some calls were
and some weren't and the acquire system would later mostly ignore the
transient flag and guess by using the FailReason instead.
Introducing a tri-state enum we can pass the information about fatal or
transient errors through the callstack to generate the correct fails.
|
| |
|
|
|
|
|
| |
If retries are enabled only transient errors are retried, which are very
few errors. At least for some HTTP codes it could be beneficial to retry
them through so adding them seems like a good idea if only to be more
consistent in what we report.
|
| |
|
|
|
|
|
|
|
| |
The casts are useless, but the reports show some where we can actually
improve the code by replacing them with better alternatives like
converting whatever int type into a string instead of casting to a
specific one which might in the future be too small.
Reported-By: gcc -Wuseless-cast
|
| |
|
|
|
|
| |
This was a left over from the autodetect move.
Gbp-Dch: ignore
|
| |
|
|
|
|
|
| |
This avoids running the Proxy-Auto-Detect script inside the
untrusted (well, less trusted for now) sandbox. This will allow
us to restrict the http method from fork()ing or exec()ing via
seccomp.
|
| |
|
|
|
|
|
|
|
|
|
| |
Opening the file before we drop privileges in the methods allows us to
avoid chowning in the acquire main process which can apply to the wrong
file (imagine Binary scoped settings) and surprises users as their
permission setup is overridden.
There are no security benefits as the file is open, so an evil method
could as before read the contents of the file, but it isn't worse than
before and we avoid permission problems in this setup.
|
| |
|
|
|
|
|
| |
On HTTP Connect we since recently look into the auth.conf file for login
information, so we should really look for all proxies into the file as
the argument is the same as for sources entries and it is easier to
document (especially as the manpage already mentions it as supported).
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We have support for an netrc-like auth.conf file since 0.7.25 (closing
518473), but it was never documented in apt that it even exists and
netrc seems to have fallen out of usage as a manpage for it no longer
exists making the feature even more arcane.
On top of that the code was a bit of a mess (as it is written in c-style)
and as a result the matching of machine tokens to URIs also a bit
strange by checking for less specific matches (= without path) first.
We now do a single pass over the stanzas.
In practice early adopters of the undocumented implementation will not
really notice the differences and the 'new' behaviour is simpler to
document and more usual for an apt user.
Closes: #811181
|
| |
|
|
|
|
|
|
|
| |
Failing on too much data is good, but we can do better by checking for
exact filesizes as we know with hashsums how large a file should be, so
if we get a file which has a size we do not expect we can drop it
directly, regardless of if the file is larger or smaller than what we
expect which should catch most cases which would end up as hashsum
errors later now a lot sooner.
|
| |
|
|
|
| |
It is highly unlikely to encounter fields which start with HTTP in
practice, but we should really be a bit more restrictive here.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
This makes it easier to see which headers includes what.
The changes were done by running
git grep -l '#\s*include' \
| grep -E '.(cc|h)$' \
| xargs sed -i -E 's/(^\s*)#(\s*)include/\1#\2 include/'
To modify all include lines by adding a space, and then running
./git-clang-format.sh.
|
| |
|
|
|
|
| |
APT considered any response with a Content-Length to have a
body, even if the value of the header was 0. A 0 length body
however, is equal to no body.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
An unknown code should be handled the same as the x00 code of this
group, but for redirections we used to treat 300 (and a few others)
as an error while unknown codes were considered redirections.
Instead we check now explicitly for the redirection codes we support for
redirecting (and add the 308 defined in RFC 7538) to avoid future
problems if new 3xx codes are added expecting certain behaviours.
Potentially strange would have been e.g. "305 Use Proxy" sending a
Location for the proxy to use – which wouldn't have worked and resulted
in an error anyhow, but probably confused users in the process.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This fixes issues with sourceforge where the redirector includes
such a Content-Range in a 302 redirect. Since we do not really know
what file is meant in a redirect, let's just ignore it for all
responses other than 416 and 206.
Maybe we should also get rid of the other errors, and just ignore
the field in those cases as well?
LP: #1657567
|
| |
|
|
|
|
|
|
| |
Most of them in (old) code comments. The two instances of user visible
string changes the po files of the manpages are fixed up as well.
Gbp-Dch: Ignore
Reported-By: spellintian
|
|
|
This 'method' is the abstract base for http and https and should as such
be called out like this rather using an easily confused name.
Gbp-Dch: Ignore
|
