summaryrefslogtreecommitdiff
path: root/apt-pkg/acquire-item.cc
Commit message (Collapse)AuthorAgeFilesLines
* prevent C++ locale number formatting in text APIs (try 2)David Kalnischkies2016-07-301-1/+1
| | | | | | | Followup of b58e2c7c56b1416a343e81f9f80cb1f02c128e25. Still a regression of sorts of 8b79c94af7f7cf2e5e5342294bc6e5a908cacabf. Closes: 832044
* rred: truncate result file before writing to itDavid Kalnischkies2016-07-271-13/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | If another file in the transaction fails and hence dooms the transaction we can end in a situation in which a -patched file (= rred writes the result of the patching to it) remains in the partial/ directory. The next apt call will perform the rred patching again and write its result again to the -patched file, but instead of starting with an empty file as intended it will override the content previously in the file which has the same result if the new content happens to be longer than the old content, but if it isn't parts of the old content remain in the file which will pass verification as the new content written to it matches the hashes and if the entire transaction passes the file will be moved the lists/ directory where it might or might not trigger errors depending on if the old content which remained forms a valid file together with the new content. This has no real security implications as no untrusted data is involved: The old content consists of a base file which passed verification and a bunch of patches which all passed multiple verifications as well, so the old content isn't controllable by an attacker and the new one isn't either (as the new content alone passes verification). So the best an attacker can do is letting the user run into the same issue as in the report. Closes: #831762
* verify hash of input file in rredDavid Kalnischkies2016-07-261-3/+6
| | | | | | | | | | | | We read the entire input file we want to patch anyhow, so we can also calculate the hash for that file and compare it with what he had expected it to be. Note that this isn't really a security improvement as a) the file we patch is trusted & b) if the input is incorrect, the result will hardly be matching, so this is just for failing slightly earlier with a more relevant error message (althrough, in terms of rred its ignored and complete download attempt instead).
* allow arch=all to override No-Support-for-Architecture-allDavid Kalnischkies2016-07-221-2/+1
| | | | | | | | | | | | | If a user explicitly requests the download of arch:all apt shouldn't get in the way and perform its detection dance if arch:all packages are (also) in arch:any files or not. This e.g. allows setting arch=all on a source with such a field (or one which doesn't support all at all, but has the arch:all files like Debian itself ATM) to get only the arch:all packages from there instead of behaving like a no-op. Reported-By: Helmut Grohne on IRC
* use +0000 instead of UTC by default as timezone in outputDavid Kalnischkies2016-07-021-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | All apt versions support numeric as well as 3-character timezones just fine and its actually hard to write code which doesn't "accidently" accepts it. So why change? Documenting the Date/Valid-Until fields in the Release file is easy to do in terms of referencing the datetime format used e.g. in the Debian changelogs (policy §4.4). This format specifies only the numeric timezones through, not the nowadays obsolete 3-character ones, so in the interest of least surprise we should use the same format even through it carries a small risk of regression in other clients (which encounter repositories created with apt-ftparchive). In case it is really regressing in practice, the hidden option -o APT::FTPArchive::Release::NumericTimezone=0 can be used to go back to good old UTC as timezone. The EDSP and EIPP protocols use this 'new' format, the text interface used to communicate with the acquire methods does not for compatibility reasons even if none of our methods would be effected and I doubt any other would (in these instances the timezone is 'GMT' as that is what HTTP/1.1 requires). Note that this is only true for apt talking to methods, (libapt-based) methods talking to apt will respond with the 'new' format. It is therefore strongly adviced to support both also in method input.
* imbue .diff/Index parsing with C.UTF-8 as wellDavid Kalnischkies2016-06-271-0/+5
| | | | | | | In 3bdff17c894d0c3d0f813d358fc45d7a263f3552 we did it for the datetime parsing, but we use the same style in the parsing for pdiff (where the size of the file is in the middle of the three fields) so imbueing here as well is a good idea.
* add insecure (and weak) allow-options for sources.listDavid Kalnischkies2016-06-221-9/+43
| | | | | | | | Weak had no dedicated option before and Insecure and Downgrade were both global options, which given the effect they all have on security is rather bad. Setting them for individual repositories only isn't great but at least slightly better and also more consistent with other settings for repositories.
* add [weak] tag to hash errors to indicate insufficiencyDavid Kalnischkies2016-06-221-2/+13
| | | | | | | For "Hash Sum mismatch" that info doesn't make a whole lot of difference, but for the new insufficient info message an indicator that while this hashes are there and even match, they aren't enough from a security standpoint.
* better error message for insufficient hashsumsDavid Kalnischkies2016-06-221-1/+7
| | | | | | | | Downloading and saying "Hash Sum mismatch" isn't very friendly from a user POV, so with this change we try to detect such cases early on and report it, preferably before download even started. Closes: 827758
* generalize secure->insecure downgrade protectionDavid Kalnischkies2016-06-221-49/+59
| | | | | | | Handling the extra check (and force requirement) for downgrades in security in our AllowInsecureRepositories checker helps in having this check everywhere instead of just in the most common place and requiring a little extra force in such cases is always good.
* handle weak-security repositories as unauthenticatedDavid Kalnischkies2016-06-221-17/+50
| | | | | | | | | | | | | | | | APT can be forced to deal with repositories which have no security features whatsoever, so just giving up on repositories which "just" fail our current criteria of good security features is the wrong incentive. Of course, repositories are better of fixing their setup to provide the minimum of security features, but sometimes this isn't possible: Historic repositories for example which do not change (anymore). That also fixes problem with repositories which are marked as trusted, but are providing only weak security features which would fail the parsing of the Release file. Closes: 827364
* fix two typos in untranslated errors of libapt-pkgDavid Kalnischkies2016-05-241-3/+3
| | | | | Reported-By: lintian: spelling-error-in-binary Git-Dch: Ignore
* implement Fallback-Of for IndexTargetsDavid Kalnischkies2016-05-081-1/+13
| | | | | | | | | | | | | | | | Sometimes index files are in different locations in a repository as it is currently the case for Contents files which are per-component in Debian, but aren't in Ubuntu. This has historic reasons and is perhaps changed soon, but such cases of transitions can always happen in the future again, so we should prepare: Introduced is a new field declaring that the current item should only be downloaded if the mentioned item wasn't allowing for transitions without a flagday in clients and archives. This isn't implemented 'simpler' with multiple MetaKeys as items (could) change their descriptions and perhaps also other configuration bits with their location.
* don't construct MetaIndex acquire items with IndexTargetsDavid Kalnischkies2016-05-071-44/+38
| | | | | | | | We don't have to initialize the Release files with a set of IndexTargets to acquire, but instead wait for the Release file to be acquired and only then ask which IndexTargets to get. Git-Dch: Ignore
* delay progress until Release files are downloadedDavid Kalnischkies2016-05-071-12/+11
| | | | | | | | | | | | Progress reporting used an "upper bound" on files we might get, expect that this wasn't correct in case pdiff entered the picture. So instead of calculating a value which is perhaps incorrect, we just accept that we can't tell how many files we are going to download and just keep at 0% until we know. Additionally, if we have pdiffs we wait until we got these (sub)index files, too. That could all be done better by downloading all Release files first and planing with them in hand accordingly, but one step at a time.
* TransactionManager can never be a nullptrDavid Kalnischkies2016-05-071-18/+11
| | | | | | | | The code naturally evolved from a TransactionManager optional to a required setup which resulted in various places doing unneeded checks suggesting a more complicated setup than is actually needed. Git-Dch: Ignore
* fix same-mirror redirection for Release{,.gpg} pairDavid Kalnischkies2016-05-071-2/+2
| | | | | | | | | | | Commit 9b8034a9fd40b4d05075fda719e61f6eb4c45678 just deals with InRelease properly and generates broken URIs in case the mirror (or the achieve really) has no InRelease file. [As this was in no released version no need to clutter changelog with a fix notice.] Git-Dch: Ignore
* don't show NO_PUBKEY warning if repo is signed by another keyDavid Kalnischkies2016-05-011-19/+2
| | | | | | | | | | | | | | | Daniel Kahn Gillmor highlights in the bugreport that security isn't improving by having the user import additional keys – especially as importing keys securely is hard. The bugreport was initially about dropping the warning to a notice, but in given the previously mentioned observation and the fact that we weren't printing a warning (or a notice) for expired or revoked keys providing a signature we drop it completely as the code to display a message if this was the only key is in another path – and is considered critical. Closes: 618445
* use the same redirection mirror for all index filesDavid Kalnischkies2016-04-251-0/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Redirection services like httpredir.debian.org tend to use a set of mirrors from which they pick a mirror at "random" for each requested file, which is usually benefitial for the download of debs, but for the index files this can quickly cause problems (aka hashsum mismatches) if the two (or more) mirrors involved are only slightly out-of-sync. This commit "resolves" this issue by using the mirror we ended up using to get the (signed) Release file directly to get the index files belonging to this Release file instead of asking the redirection service which eliminates the risk of hitting out-of-sync mirrors. As an obvious downside the redirection service can't serve partial mirrors anymore for indexes and the download of indexes indexed in the same Release file can't be done in parallel (from different mirrors). This does not effect the download of non-index files like deb-files as out-of-sync mirrors aren't a huge problem there, so the parallel download outweights a potentially 404 error (also because this causes no errenous downloads while hashsum mismatches download the entire file before finding out that it was pointless). The rational for this is that indexes are relative to the Release file. If we would be talking about a HTML page including images, such a behaviour is obvious and intended – not doing it means in the best case a bunch of "useless" requests which will all be answered with a redirect.
* show more details for "Writing more data" errors, tooDavid Kalnischkies2016-04-251-21/+42
| | | | | | They are the small brothers of the hashsum mismatch, so they deserve a similar treatment even through we have for architectual reasons not a much to display as for hashsum mismatches for now.
* show more details for "Hash Sum mismatch" errorsDavid Kalnischkies2016-04-251-13/+47
| | | | | | | | | | | | | | | | Users tend to report these errors with just this error message… not very actionable and hard to figure out if this is a temporary or 'permanent' mirror-sync issue or even the occasional apt bug. Showing the involved hashsums and modification times should help in triaging these kind of bugs – and eventually we will have less of them via by-hash. The subheaders aren't marked for translation for now as they are technical glibberish and probably easier to deal with if not translated. After all, our iconic "Hash Sum mismatch" is translated at least. These additions were proposed in #817240 by Peter Palfrader.
* sanify unused ReportMirrorFailure a tiny bitDavid Kalnischkies2016-04-251-62/+68
| | | | | | | | | | Calling the (non-existent) reporter multiple times for the same error with different codes for the same error (e.g. hashsum) is a bit strange. It also doesn't need to be a public API. Ideally that would all look and behave slightly different, but we will worry about that at the time this is actually (planed to be) used somewhere… Git-Dch: Ignore
* ensure outdated files are dropped without lists-cleanupDavid Kalnischkies2016-04-141-3/+51
| | | | | Tested via (newly) empty index files, but effects also files dropped from the repository or an otherwise changed repository config.
* silently skip acquire of empty index filesDavid Kalnischkies2016-04-141-3/+9
| | | | | There is just no point in taking the time to acquire empty files – especially as it will be tiny non-empty compressed files usually.
* fix Alt-Filename handling of file methodDavid Kalnischkies2016-04-141-5/+4
| | | | | | | | | | A silly of-by-one error in the stripping of the extension to check for the uncompressed filename broken in an attempt to support all compressions in commit a09f6eb8fc67cd2d836019f448f18580396185e5. Fixing this highlights also mistakes in the handling of the Alt-Filename in libapt which would cause apt to remove the file from the repository (if root has the needed rights – aka the disk isn't readonly or similar)
* stop handling items in doomed transactionsDavid Kalnischkies2016-04-071-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | With the previous commit we track the state of transactions, so we can now use our knowledge to avoid processing data for a transaction which was already closed (via an abort in this case). This is needed as multiple independent processes are interacting in the process, so there isn't a simple immediate full-engine stop and it would also be bad to teach each and every item how to check if its manager has failed subordinate and what to do in that case. In the pdiff case, which deals (potentially) with many items during its lifetime e.g. a hashsum mismatch in another file can abort the transaction the file we try to patch via pdiff belongs to. This causes some of the items (which are already done) to be aborted with it, but items still in the process of acquisition continue in the processing and will later try to use all the items together failing in strange ways as cleanup already happened. The chosen solution is to dry up the communication channels instead by ignoring new requests for data acquisition, canceling requests which are not assigned to a queue and not calling Done/Failed on items anymore. This means that e.g. already started or pending (e.g. pipelined) downloads aren't stopped and continue as normal for now, but they remain in partial/ and aren't processed further so the next update command will pick them up and put them to good use while the current process fails updating (for this transaction group) in an orderly fashion. Closes: 817240 Thanks: Barr Detwix & Vincent Lefevre for log files
* ensure transaction states are changed only onceDavid Kalnischkies2016-04-071-10/+29
| | | | | | | | | | | | | | We want to keep track of the state of a transaction overall to base future decisions on it, but as a pre-requirement we have to make sure that a transaction isn't commited twice (which happened if the download of InRelease failed and Release takes over). It also happened to create empty commits after a transaction was already aborted in cases in which the Release files were rejected. This isn't effecting security at the moment, but to ensure this isn't happening again and can never be bad a bunch of fatal error messages are added to make regressions on this front visible.
* refactor loading of previous release fileDavid Kalnischkies2016-03-191-66/+29
| | | | | | There is really no need to have the same code three times. Git-Dch: Ignore
* Get accurate progress reporting in apt update againMichael Vogt2016-03-161-0/+4
| | | | | | | | | | | | For the non-pdiff case, we have can have accurate progress reporting because after fetching the {,In}Release files we know how many IndexFiles will be fetched and what size they have. Therefore init the filesize early (in pkgAcqIndex::Init) and ensure that in Acquire::Pulse() looks at already downloaded bits when calculating the progress in Acquire::Pulse. Also improve debug output of Debug::acquire::progress
* don't use Desc.URI to calculate .diff/Index filenamesDavid Kalnischkies2016-03-141-11/+25
| | | | | | | The URI descibing an item can change via mirrors/redirectors which causes the .diff/Index files to get the wrong names in storage. Git-Dch: Ignore
* require $(HASH)-Download field in .diff/Index filesDavid Kalnischkies2016-03-141-24/+17
| | | | | | | | | | | | Now that we ignore SHA1-only files it makes sense to require also the provision of hashes for the compressed patches as this was introduced in the same patchset as support for non-SHA1 hashes in the file itself in dak and adding support in other archive creators (if they support pdiffs at all) will likely be in the same batch. The reason for the change itself is simple: If you are 'scared' enough about the security of SHA1, you shouldn't uncompress a file you haven't verified at all – after all, it could be exploiting a bug or a zip bomb.
* Fix several typosVeres Lajos2016-03-071-2/+2
| | | | | | | | | | | | | This effectively merges branch 'typofixes-vlajos-20150807' of github.com:vlajos/apt with the following commit: commit 13cacb3e2e2352ba701e769fc889e3344fabbf7e Author: Veres Lajos <vlajos@gmail.com> Date: Sun Aug 9 00:12:53 2015 +0100 typofix - https://github.com/vlajos/misspell_fixer It has been rebased for a better commit message.
* do not move not-failed pdiff-patches into CWD on failureDavid Kalnischkies2016-03-061-0/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | If a single pdiff fails, we have to fail the entire patching endeavour and fall back to getting the complete file instead. That is easy in serverside merged pdiffs as we get them one by one. For clientside we get them all at once through, which means that a failure in one has to stop the entire pipeline, which works as expected (as proven by the bugreporters as they don't even notice it happening). The problem is just that the first failing pdiff will do the cleanup, so another pdiff which happens to be successfully acquired after we processed the failure doesn't find the file it is supposed to use as a basename anymore, so the patch is renamed to what should be the unique extension and moved into the current working directory. Processing is then stopped as the patch realizes that it isn't the last one which completed downloading. On the plus side this means this is neither us using a bad temporary location nor a security problem. It "just" overrides unconditionally files in your current working directory (if you happen to have them named like a pdiff patch – a bit unlikely perhaps) and so drops files there which are never used again. I guess this was introduced in 4e3c5633b1e74b4f58b95f339cfbbf4cbf21ab3e for real as I made the need for the existence of the base file rather explicit, but the potential lingers in the code for far longer. Closes: #816837
* deal with partially downloaded changelogsDavid Kalnischkies2016-03-061-2/+19
| | | | | | | | | | | | Changelogs are relatively small and we have no hashes for them, but we had partial support for them before, so lets stick to it. This also deletes the (partial) file before moving the downloaded file into its place – rename(2) should be doing this by itself, but testing on semaphoreci suggests that this isn't always the case (error is "Stale file handle") and we don't need an atomic replace here, so be explicit. Git-Dch: Ignore
* Add missing numeric includes in files using std::accumulate()Julian Andres Klode2016-02-261-0/+1
| | | | Reported-By: Helmut Grohne on IRC
* always download changelogs into /tmp firstDavid Kalnischkies2016-02-111-24/+32
| | | | | | | | | | pkgAcqChangelog has the default behaviour of downloading a changelog to a temporary directory (inside /tmp, not /tmp directly), which is cleaned up on shutdown, but this can be overridden to store the changelog more permanently – but that caries a permission problem. For changelog we can 'easily' solve this by always downloading to a temporary directory and only move it out of there on done.
* use local changelog from /usr/share/doc if possibleDavid Kalnischkies2016-02-111-2/+33
| | | | | | | | | | | If pkgAcqChangelog is told to acquire the changelog for a version it will check first if this version is installed on the disk and if so will use the local changelog in /usr/share/doc (possibily/likely gz compressed) instead of downloading the file from the web. An option is provided to disable this, which is enabled by default for the Ubuntu vendor as they truncate the local changelogs – and for apts --print-uris action.
* remove uncompressed leftover partial file before pdiff bootstrapDavid Kalnischkies2016-01-081-8/+21
| | | | | | | The code already deals with compressed leftovers, but forgot the uncompressed files. The opertunity is picked to reorder this code and add debug messages about the actions taken as well as produce such a leftover file in the associated testcase.
* use filesize of compressed pdiffs for the limit if possibleDavid Kalnischkies2016-01-081-12/+49
| | | | | | | | | | | | With the addition of the $HASH-Download field in the .diff/Index we got the size of the compressed patches for 'free', so if that information is available we can use it for a more fitting calculation of the size requirements of the patches vs. the complete file. Note that this predicts a too small size in the transition case in which the information isn't available for all patches, but figuring this out would be a lot of code for practically nothing as only one update can ever be in such a transition phase.
* keep compressed indexes in a low-cost formatDavid Kalnischkies2016-01-081-99/+78
| | | | | | | | | | | | | | | | | | | | | | | | | | | Downloading and storing are two different operations were different compression types can be preferred. For downloading we provide the choice via Acquire::CompressionTypes::Order as there is a choice to be made between download size and speed – and limited by whats available in the repository. Storage on the other hand has all compressions currently supported by apt available and to reduce runtime of tools accessing these files the compression type should be a low-cost format in terms of decompression. apt traditionally stores its indexes uncompressed on disk, but has options to keep them compressed. Now that apt downloads additional files we also deal with files which simply can't be stored uncompressed as they are just too big (like Contents for apt-file). Traditionally they are downloaded in a low-cost format (gz) as repositories do not provide other formats, but there might be even lower-cost formats and for download we could introduce higher-cost in the repositories. Downloading an entire index potentially requires recompression to another format, so an update takes potentially longer – but big files are usually updated via pdiffs which has to de- and re-compress anyhow and does it on the fly anyhow, so there is no extra time needed and in general it seems to be benefitial to invest the time in update to save time later on file access.
* allow pdiff bootstrap from all supported compressorsDavid Kalnischkies2016-01-081-177/+140
| | | | | | | There is no reason to enforce that the file we start the bootstrap with is compressed with a compressor which is available online. This allows us to change the on-disk format as well as deals with repositories adding/removing support for a specific compressor.
* ensure compression cleanup even without lists-cleanupDavid Kalnischkies2016-01-081-5/+32
| | | | | | | | | | | | If we store files compressed in lists/ and the file switched compression formats we happened to retain the "old" format, but by default the cleanup process catched this oversight and removed the file. [The initial situation described doesn't arise as we store no files by default compressed and even with apt-file configuring Contents files, we don't really have that problem as there is just .gz files for those.] We solve this by just removing any uncompressed as well as compressed (we support) file just before we move the 'new' version of the file in.
* use one 'store' method to rule all (de)compressorsDavid Kalnischkies2016-01-081-15/+22
| | | | | | | | | | | Adding a new compressor method meant adding a new method as well – even if that boilt down to just linking to our generalized decompressor with a new name. That is unneeded busywork if we can instead just call the generalized decompressor and let it figure out which compressor to use based on the filenames rather than by program name. For compatibility we ship still 'gzip', 'bzip2' and co, but they are just links to our "new" 'store' method.
* allow repositories to forbid arch:all for specific index targetsDavid Kalnischkies2015-12-271-3/+7
| | | | | | | | | | | | | | | | | | | | | | | | Debian has a Packages file for arch:all already, but the arch:any files contain arch:all packages as well, so downloading it would be a total waste of resources. Getting this solved is on the list of things to do, but it is also the hardest part – for index targets like Contents the situation is much easier and less server/client implementations are involved so we might not want to stall them. A repository can now declare via: No-Support-for-Architecture-all: Packages that even if an arch:all Packages exists, it shouldn't be downloaded, so that support for Contents files can be added now. See also 1dd20368486820efb6ef4476ad739e967174bec4 for the implementation of downloading arch:all index targets, which this is limiting. The field uses the name of the target from the apt configuration for simplicity and is negative by design as this field is intended to be supported/needed only for a "short" time (one or two Debian releases). While this commit theoretically supports any target, its expected to only see "Packages" as a value in reality.
* show a more descriptive error for weak Release filesDavid Kalnischkies2015-12-141-0/+10
| | | | | | | | | | | | | | If we can't work with the hashes we parsed from the Release file we display now an error message if the Release file includes only weak hashes instead of downloading the indexes and failing to verify them with "Hash Sum mismatch" even through the hashes didn't mismatch (they were just weak). If for some (unlikely) reason we have got weak hashes only for individual targets we will show a warning to this effect (again, befor downloading and failing the index itself). Closes: 806459
* parse .diff/Index hashes in reverse orderDavid Kalnischkies2015-12-131-3/+11
| | | | | | | | | | | | | | | | Reversing the parsing order ensures that we parse weaker hashes (like SHA1) before we touch newer/stronger hashes (like SHA256) as the weaker ones will usually be there for a longer time already with data already present, which we would discard if we start with the strong one first. The discarding is visible in the debug logs: File X wasn't in the list for the first parsed hash! (history) File X wasn't in the list for the first parsed hash! (patches) which if file X is part of the patch-path means apt will not find a path and fallback to acquire the whole file instead needlessly. If file X isn't part of the patch-path that is no problem, so that effects only the update-call which updates with patches coming from before and after the addition of a new hash.
* use @CHANGEPATH@ as placeholder in changelog URI templatesDavid Kalnischkies2015-12-021-2/+2
| | | | | | | | | This should make it more obvious that CHANGEPATH is a placeholder which apt will replace with a package specific path rather than a string constant. Mail-Reference: <87d1upgvaf.fsf@deep-thought.43-1.org> Mail-Archive: https://lists.debian.org/debian-dak/2015/12/msg00005.html
* slightly rephrase notice shown for insecure repositoriesJustin B Rye2015-11-251-1/+1
| | | | Git-Dch: Ignore
* review of new/changed translatable program stringsJustin B Rye2015-11-211-5/+5
| | | | | Reference mail: https://lists.debian.org/debian-l10n-english/2015/11/msg00006.html
* do not sent Last-Modified if we expect a changed fileDavid Kalnischkies2015-11-211-4/+12
| | | | | | | | | | | | | | | | | | | | In 8d041b4f we made apt figure out based on the last Release file it has if it should request a file or not given that the hashes changed or not. So if we have a last Release file and do a request, do not sent a Last-Modified header as we expect a change so much that a non-change would indeed be an error. The Last-Modified header is therefore at best ignored by the server, so sending it is just wasted effort. In the worst case as time is a fragile thing the server decides against sending us an update with the idea that we already have the latest content, which we know for a fact that we haven't. Given that we sent less information to the server our request is on its own also less identifiable as coming from a returning or new user. The disadvantage is that if we end up getting an old index file after getting a new Release file from another mirror the old mirror will not be able to tell us 'Hit', but instead sends us the complete file we discard, but both lets us end up with the same error class in the end, so the difference isn't big in practice.