[#7875] Fix attachment processing #7932
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gbp
force-pushed
the
attachment-processing-reparse
branch
from
bc931f5
to
00f734b
Compare
We're going to make a change which might mean attachments might be rebuilt within the `FoiAttachmentMaskJob` this means the signed global ID might not return the attachment in this controller. This change ensures their is a referer parameter and if the attachment can't be loaded then we redirect back to the referer. Note, we don't need to worry about people modifying the referer to get users redirected to a third party domain as this is protected against by Rails.
This check isn't doing anything. It was meant to protect when parsing incoming messages to stop hidden attachments from being made public again. We don't actually yet have this parsing in place instead we've focused on finding the original attachment [1]. For the remainder of cases we will need to parse the raw email again but we have added separate check [2] so this one here is redundant. [1] #7875 [2] #7920
Catch and rescue from `MissingAttachment` exceptions. Re-parse the incoming message to rebuild the attachments instead. Fixes: #7875
When attachment is unmasked we attempt to run the mask job inline. This commit makes a couple of changes. 1. It unlocks the job so we can run it, so we don't have to worry about if the job doesn't run 2. Now the mask job can call `IncomingMessage#parse_raw_email!(true)` we need to handle to case where the attachment disappears after being rebuilt. This results in a `ActiveRecord::RecordNotFound` when reloading the instance. 3. Add RebuiltAttachment exception which we can then catch and rescue later. See the next commit.
When calling `FoiAttachment#body` this can now raise an exception due to the current attachment instance getting replaced and no longer existing in the database. We can catch and rescue from this by reloading the new attachment and calling the same code again. This is what `FoiAttachment.protect_against_rebuilt_attachments` does and should prevent exceptions when viewing requests which haven't be masked and cached yet. These will normally be very old attachments as this also need the `hexdigest` to be mismatched.
Exception notifications don't include the name of the exception so improve the message to make it clearer what the issue is.
gbp
force-pushed
the
attachment-processing-reparse
branch
from
00f734b
to
625c685
Compare
This was referenced Oct 3, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relevant issue(s)
Fixes: #7875
What does this do?
IncomingMessage#parse_raw_email!(true)when the original attachment body can't be retrieved. This should rebuild the attachment in the database and correct the mis-matched hexdigest which allows the original attachment to be found again in the future.request#showand other actions where the attachment can be masked for the first time.Why was this needed?
To ensure all attachments are viewable and accessible by users.