summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--apt-pkg/acquire-item.cc69
-rw-r--r--apt-pkg/acquire-item.h5
-rw-r--r--apt-pkg/acquire-worker.cc10
-rw-r--r--methods/aptmethod.h3
-rw-r--r--methods/http.cc6
-rw-r--r--methods/http.h1
-rw-r--r--methods/https.h1
-rw-r--r--methods/server.cc75
-rw-r--r--methods/server.h1
-rwxr-xr-xtest/integration/test-apt-redirect-loop23
10 files changed, 129 insertions, 65 deletions
diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index ad8cb7f24..f13d2f6ae 100644
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -685,10 +685,15 @@ class APT_HIDDEN CleanupItem : public pkgAcqTransactionItem /*{{{*/
/*}}}*/
// Acquire::Item::Item - Constructor /*{{{*/
+class pkgAcquire::Item::Private
+{
+public:
+ std::vector<std::string> PastRedirections;
+};
APT_IGNORE_DEPRECATED_PUSH
pkgAcquire::Item::Item(pkgAcquire * const owner) :
FileSize(0), PartialSize(0), Mode(0), ID(0), Complete(false), Local(false),
- QueueCounter(0), ExpectedAdditionalItems(0), Owner(owner), d(NULL)
+ QueueCounter(0), ExpectedAdditionalItems(0), Owner(owner), d(new Private())
{
Owner->Add(this);
Status = StatIdle;
@@ -699,6 +704,7 @@ APT_IGNORE_DEPRECATED_POP
pkgAcquire::Item::~Item()
{
Owner->Remove(this);
+ delete d;
}
/*}}}*/
std::string pkgAcquire::Item::Custom600Headers() const /*{{{*/
@@ -766,32 +772,40 @@ void pkgAcquire::Item::Failed(string const &Message,pkgAcquire::MethodConfig con
}
string const FailReason = LookupTag(Message, "FailReason");
- enum { MAXIMUM_SIZE_EXCEEDED, HASHSUM_MISMATCH, WEAK_HASHSUMS, OTHER } failreason = OTHER;
+ enum { MAXIMUM_SIZE_EXCEEDED, HASHSUM_MISMATCH, WEAK_HASHSUMS, REDIRECTION_LOOP, OTHER } failreason = OTHER;
if ( FailReason == "MaximumSizeExceeded")
failreason = MAXIMUM_SIZE_EXCEEDED;
else if ( FailReason == "WeakHashSums")
failreason = WEAK_HASHSUMS;
+ else if (FailReason == "RedirectionLoop")
+ failreason = REDIRECTION_LOOP;
else if (Status == StatAuthError)
failreason = HASHSUM_MISMATCH;
if(ErrorText.empty())
{
+ std::ostringstream out;
+ switch (failreason)
+ {
+ case HASHSUM_MISMATCH:
+ out << _("Hash Sum mismatch") << std::endl;
+ break;
+ case WEAK_HASHSUMS:
+ out << _("Insufficient information available to perform this download securely") << std::endl;
+ break;
+ case REDIRECTION_LOOP:
+ out << "Redirection loop encountered" << std::endl;
+ break;
+ case MAXIMUM_SIZE_EXCEEDED:
+ out << LookupTag(Message, "Message") << std::endl;
+ break;
+ case OTHER:
+ out << LookupTag(Message, "Message");
+ break;
+ }
+
if (Status == StatAuthError)
{
- std::ostringstream out;
- switch (failreason)
- {
- case HASHSUM_MISMATCH:
- out << _("Hash Sum mismatch") << std::endl;
- break;
- case WEAK_HASHSUMS:
- out << _("Insufficient information available to perform this download securely") << std::endl;
- break;
- case MAXIMUM_SIZE_EXCEEDED:
- case OTHER:
- out << LookupTag(Message, "Message") << std::endl;
- break;
- }
auto const ExpectedHashes = GetExpectedHashes();
if (ExpectedHashes.empty() == false)
{
@@ -822,10 +836,8 @@ void pkgAcquire::Item::Failed(string const &Message,pkgAcquire::MethodConfig con
}
out << "Last modification reported: " << LookupTag(Message, "Last-Modified", "<none>") << std::endl;
}
- ErrorText = out.str();
}
- else
- ErrorText = LookupTag(Message,"Message");
+ ErrorText = out.str();
}
switch (failreason)
@@ -833,6 +845,7 @@ void pkgAcquire::Item::Failed(string const &Message,pkgAcquire::MethodConfig con
case MAXIMUM_SIZE_EXCEEDED: RenameOnError(MaximumSizeExceeded); break;
case HASHSUM_MISMATCH: RenameOnError(HashSumMismatch); break;
case WEAK_HASHSUMS: break;
+ case REDIRECTION_LOOP: break;
case OTHER: break;
}
@@ -976,6 +989,24 @@ std::string pkgAcquire::Item::HashSum() const /*{{{*/
return hs != NULL ? hs->toStr() : "";
}
/*}}}*/
+bool pkgAcquire::Item::IsRedirectionLoop(std::string const &NewURI) /*{{{*/
+{
+ if (d->PastRedirections.empty())
+ {
+ d->PastRedirections.push_back(NewURI);
+ return false;
+ }
+ auto const LastURI = std::prev(d->PastRedirections.end());
+ // redirections to the same file are a way of restarting/resheduling,
+ // individual methods will have to make sure that they aren't looping this way
+ if (*LastURI == NewURI)
+ return false;
+ if (std::find(d->PastRedirections.begin(), LastURI, NewURI) != LastURI)
+ return true;
+ d->PastRedirections.push_back(NewURI);
+ return false;
+}
+ /*}}}*/
pkgAcqTransactionItem::pkgAcqTransactionItem(pkgAcquire * const Owner, /*{{{*/
pkgAcqMetaClearSig * const transactionManager, IndexTarget const &target) :
diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h
index ac4994738..e6e5ea12b 100644
--- a/apt-pkg/acquire-item.h
+++ b/apt-pkg/acquire-item.h
@@ -304,6 +304,8 @@ class pkgAcquire::Item : public WeakPointable /*{{{*/
*/
virtual ~Item();
+ bool APT_HIDDEN IsRedirectionLoop(std::string const &NewURI);
+
protected:
/** \brief The acquire object with which this item is associated. */
pkgAcquire * const Owner;
@@ -357,7 +359,8 @@ class pkgAcquire::Item : public WeakPointable /*{{{*/
virtual std::string GetFinalFilename() const;
private:
- void * const d;
+ class Private;
+ Private * const d;
friend class pkgAcqMetaBase;
friend class pkgAcqMetaClearSig;
diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc
index 39cc55bdf..1ee78d070 100644
--- a/apt-pkg/acquire-worker.cc
+++ b/apt-pkg/acquire-worker.cc
@@ -269,6 +269,16 @@ bool pkgAcquire::Worker::RunMessages()
for (auto const &Owner: ItmOwners)
{
pkgAcquire::ItemDesc &desc = Owner->GetItemDesc();
+ if (Owner->IsRedirectionLoop(NewURI))
+ {
+ std::string msg = Message;
+ msg.append("\nFailReason: RedirectionLoop");
+ Owner->Failed(msg, Config);
+ if (Log != nullptr)
+ Log->Fail(Owner->GetItemDesc());
+ continue;
+ }
+
if (Log != nullptr)
Log->Done(desc);
diff --git a/methods/aptmethod.h b/methods/aptmethod.h
index d3c948636..0963ea21e 100644
--- a/methods/aptmethod.h
+++ b/methods/aptmethod.h
@@ -17,7 +17,8 @@
class aptMethod : public pkgAcqMethod
{
- char const * const Binary;
+protected:
+ std::string Binary;
public:
virtual bool Configuration(std::string Message) APT_OVERRIDE
diff --git a/methods/http.cc b/methods/http.cc
index c61ca1c3f..cf5eae06d 100644
--- a/methods/http.cc
+++ b/methods/http.cc
@@ -465,6 +465,12 @@ bool HttpServerState::RunData(FileFd * const File)
return Owner->Flush() && !_error->PendingError();
}
/*}}}*/
+bool HttpServerState::RunDataToDevNull() /*{{{*/
+{
+ FileFd DevNull("/dev/null", FileFd::WriteOnly);
+ return RunData(&DevNull);
+}
+ /*}}}*/
bool HttpServerState::ReadHeaderLines(std::string &Data) /*{{{*/
{
return In.WriteTillEl(Data);
diff --git a/methods/http.h b/methods/http.h
index ac5f52314..2ac3b8c9b 100644
--- a/methods/http.h
+++ b/methods/http.h
@@ -106,6 +106,7 @@ struct HttpServerState: public ServerState
virtual void Reset() APT_OVERRIDE { ServerState::Reset(); ServerFd = -1; };
virtual bool RunData(FileFd * const File) APT_OVERRIDE;
+ virtual bool RunDataToDevNull() APT_OVERRIDE;
virtual bool Open() APT_OVERRIDE;
virtual bool IsOpen() APT_OVERRIDE;
diff --git a/methods/https.h b/methods/https.h
index 8592570c6..85cc7824e 100644
--- a/methods/https.h
+++ b/methods/https.h
@@ -39,6 +39,7 @@ class HttpsServerState : public ServerState
/** \brief Transfer the data from the socket */
virtual bool RunData(FileFd * const /*File*/) APT_OVERRIDE { return false; }
+ virtual bool RunDataToDevNull() APT_OVERRIDE { return false; }
virtual bool Open() APT_OVERRIDE { return false; }
virtual bool IsOpen() APT_OVERRIDE { return false; }
diff --git a/methods/server.cc b/methods/server.cc
index 6d147fe12..672a3d16d 100644
--- a/methods/server.cc
+++ b/methods/server.cc
@@ -297,12 +297,33 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
else
NextURI.clear();
NextURI.append(DeQuoteString(Server->Location));
+ if (Queue->Uri == NextURI)
+ {
+ SetFailReason("RedirectionLoop");
+ _error->Error("Redirection loop encountered");
+ if (Server->HaveContent == true)
+ return ERROR_WITH_CONTENT_PAGE;
+ return ERROR_UNRECOVERABLE;
+ }
return TRY_AGAIN_OR_REDIRECT;
}
else
{
NextURI = DeQuoteString(Server->Location);
URI tmpURI = NextURI;
+ if (tmpURI.Access == "http" && Binary == "https+http")
+ {
+ tmpURI.Access = "https+http";
+ NextURI = tmpURI;
+ }
+ if (Queue->Uri == NextURI)
+ {
+ SetFailReason("RedirectionLoop");
+ _error->Error("Redirection loop encountered");
+ if (Server->HaveContent == true)
+ return ERROR_WITH_CONTENT_PAGE;
+ return ERROR_UNRECOVERABLE;
+ }
URI Uri = Queue->Uri;
// same protocol redirects are okay
if (tmpURI.Access == Uri.Access)
@@ -486,10 +507,6 @@ bool ServerMethod::Fetch(FetchItem *)
// ServerMethod::Loop - Main loop /*{{{*/
int ServerMethod::Loop()
{
- typedef vector<string> StringVector;
- typedef vector<string>::iterator StringVectorIterator;
- map<string, StringVector> Redirected;
-
signal(SIGTERM,SigTerm);
signal(SIGINT,SigTerm);
@@ -720,46 +737,16 @@ int ServerMethod::Loop()
File = 0;
break;
}
-
- // Try again with a new URL
- case TRY_AGAIN_OR_REDIRECT:
- {
- // Clear rest of response if there is content
- if (Server->HaveContent)
- {
- File = new FileFd("/dev/null",FileFd::WriteExists);
- Server->RunData(File);
- delete File;
- File = 0;
- }
-
- /* Detect redirect loops. No more redirects are allowed
- after the same URI is seen twice in a queue item. */
- StringVector &R = Redirected[Queue->DestFile];
- bool StopRedirects = false;
- if (R.empty() == true)
- R.push_back(Queue->Uri);
- else if (R[0] == "STOP" || R.size() > 10)
- StopRedirects = true;
- else
- {
- for (StringVectorIterator I = R.begin(); I != R.end(); ++I)
- if (Queue->Uri == *I)
- {
- R[0] = "STOP";
- break;
- }
-
- R.push_back(Queue->Uri);
- }
-
- if (StopRedirects == false)
- Redirect(NextURI);
- else
- Fail();
-
- break;
- }
+
+ // Try again with a new URL
+ case TRY_AGAIN_OR_REDIRECT:
+ {
+ // Clear rest of response if there is content
+ if (Server->HaveContent)
+ Server->RunDataToDevNull();
+ Redirect(NextURI);
+ break;
+ }
default:
Fail(_("Internal error"));
diff --git a/methods/server.h b/methods/server.h
index af0914b9c..b23b0e50a 100644
--- a/methods/server.h
+++ b/methods/server.h
@@ -91,6 +91,7 @@ struct ServerState
/** \brief Transfer the data from the socket */
virtual bool RunData(FileFd * const File) = 0;
+ virtual bool RunDataToDevNull() = 0;
virtual bool Open() = 0;
virtual bool IsOpen() = 0;
diff --git a/test/integration/test-apt-redirect-loop b/test/integration/test-apt-redirect-loop
new file mode 100755
index 000000000..b54f74276
--- /dev/null
+++ b/test/integration/test-apt-redirect-loop
@@ -0,0 +1,23 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+
+setupenvironment
+configarchitecture "i386"
+
+insertpackage 'stable' 'apt' 'all' '1'
+setupaptarchive --no-update
+
+echo 'alright' > aptarchive/working
+changetohttpswebserver
+webserverconfig 'aptwebserver::redirect::replace::/redirectme3/' '/redirectme/'
+webserverconfig 'aptwebserver::redirect::replace::/redirectme2/' '/redirectme3/'
+webserverconfig 'aptwebserver::redirect::replace::/redirectme/' '/redirectme2/'
+
+testfailure apthelper download-file "http://localhost:${APTHTTPPORT}/redirectme/working" httpfile
+testsuccess grep 'Redirection loop encountered' rootdir/tmp/testfailure.output
+
+testfailure apthelper download-file "https://localhost:${APTHTTPSPORT}/redirectme/working" httpsfile
+testsuccess grep 'Redirection loop encountered' rootdir/tmp/testfailure.output
generated by cgit v1.2.3 (git 2.25.1) at 2024-05-13 03:19:11 +0000