Stop the exporter naming a serial it is not sending - #191
Merged
Merged
Conversation
Both the CLI and the wizard print "serial 0PENTAGXPORT" in the notice shown before the password prompt. It is a literal, written when every install shared one serial, and left behind when they stopped. An install that drew its own is told to look for a device it will not find - and the row it does find then looks like somebody else's, which is the belief that gets it removed and takes the session with it. Exactly the defect fixed on the Android side's registered-device screen in #182. These two copies were missed, and they are worse than that one was: the sentence is shown to every CLI and wizard user on every sign-in, and it is the only place the exporter says what it registered as. Found the hard way. While testing whether a freshly drawn serial gets past Apple's 503, the run printed `serial 0PENTAGXPORT` with no identity file on disk at all - which for a moment read as the draw having failed, rather than the sentence being a lie. `FakeAccount` had no `serial` attribute, so nothing in the suite could tell a sentence that names the account from one that names a literal. It has one now, deliberately not `0PENTAGXPORT`, and `test_the_notice_names_the_serial_this_run_presents` asserts both that the drawn value appears and that the old constant does not. Checked by reinstating the literal and watching it go red.
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
1 participant
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.
cli.pyandwizard.pyboth printserial 0PENTAGXPORTin the notice shown before the password prompt. It is a hardcoded literal from when every install shared one serial, and #182 did not reach it.So an install that drew its own serial is told to look for a device that is not in its device list, and the entry it does find looks like it belongs to somebody else — which is precisely the belief that gets it removed, taking the working session with it. Same defect as the Android registered-device screen, and arguably worse: this sentence is printed to every CLI and wizard user on every sign-in, and it is the only place the exporter says what it registered as.
How it surfaced. Testing whether a freshly drawn serial gets past the current 503, the run printed
serial 0PENTAGXPORTwith nodevice-identity.jsonon disk at all. For a moment that read as the draw having failed rather than as the sentence being wrong.Why no test caught it:
FakeAccounthad noserialattribute, so no test could distinguish a sentence that names the account from one that names a literal. It has one now — deliberately not0PENTAGXPORT— andtest_the_notice_names_the_serial_this_run_presentsasserts the drawn value appears and the old constant does not. Verified by reinstating the literal and watching it fail.458 exporter tests pass.