fix: stop Parse QR Code from participating in Magic (#2610)#2613
Open
Sanjays2402 wants to merge 1 commit into
Open
fix: stop Parse QR Code from participating in Magic (#2610)#2613Sanjays2402 wants to merge 1 commit into
Sanjays2402 wants to merge 1 commit into
Conversation
Parse QR Code declared a `checks` regex that matched any JPEG, PNG, GIF, WEBP or BMP magic bytes. Magic aggregates every operation with a `checks` property and runs them, so any image input triggered a full QR parse attempt that produced a 'Could not read a QR code from the image' warning in the browser console for every image, even when no QR code was present. There is no cheap way to detect a QR code without attempting a full parse, so Parse QR Code should not participate in Magic. Users can add it manually when they know the image contains a QR code. Adds a regression test asserting Parse QR Code declares no `checks`.
Member
|
@GCHQDeveloper581 @tomdev123 @np748293 would appreciate your views here. This operation has been part of magic for quite a long time (since #515 ). It doesn't seem to be an accident - it was specifically added, so we should be making a conscious decision if we are to remove it. |
Contributor
|
I originally suggested removing it from Magic when I raised Issue #2610, and have given my rationale there (and the mitigation). However, as more instances of Magic detection running amok are found - (#2410, #2642) I'm beginning to think that the correct response is to alter Magic to catch OperationError without reporting the error, and simply to remove the operation from consideration for that stage of the processing. |
This file contains hidden or 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
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.
Description
Parse QR Codedeclared achecksregex that matched any JPEG, PNG, GIF, WEBP or BMP magic bytes.Magic._generateOpCriteria(insrc/core/lib/Magic.mjs) aggregates every operation that exposes achecksproperty and runs each against the input, so any image fed to Magic (which happens implicitly wheneverMagicis in the recipe, including via the URL/QueryString demo) triggered a full QR parse. The QR decoder then logged[BGChefWorker]: Could not read a QR code from the image.to the browser console for every image that didn't contain a QR code.This PR removes the
this.checksarray fromParseQRCode's constructor (leaving a comment that points at the issue). There is no cheap way to know an image contains a QR code without actually attempting to decode one, so it can't be a sensible Magic candidate. The operation itself is unchanged and every other operation is unaffected; only Magic's automatic dispatch stops attempting QR parsing. Users who know their image has a QR code can still addParse QR Codeto the recipe manually.Behaviour before: loading the 1px-PNG demo URL from the issue with DevTools open prints
[BGChefWorker]: Could not read a QR code from the image.Behaviour after: same URL, console is silent; Magic still recognises the input as an image and lists the usual image-render suggestions.
Existing Issue
Fixes #2610.
Screenshots
No visual changes to CyberChef. The only observable difference is the absence of a console error message; no UI is affected.
AI disclosure
This change was written with the assistance of Claude. I have reviewed the diff and the test, understand the root cause and the fix, and take responsibility for the submitted code.
Test Coverage
Adds
tests/node/tests/ParseQRCode.mjswith an API-level regression test asserting thatOperationConfig["Parse QR Code"]declares nochecks. The test fails onmasterwith the originalchecksregex and passes with this change (verified both directions). The full node and operations suites still pass locally (255 and 2057 tests respectively).