Do not use AlphaNum when string contains comma

[eigan]

Proposed changes

Using dash in this context will create a range between + and . which allows the following characters: +, ,, -, ..

The ASCII range is as follow: + (43), , (44), - (45), . (46).

Comma is not supported and will then crash in write().

Types of changes

What types of changes does your code introduce?

• Bugfix (non-breaking change which fixes an issue)

Checklist:

• I have checked to ensure there aren't other open Issues or Pull Requests for the same update/change
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)
• Any dependent changes have been merged and published in downstream modules
• Static analysis and unit tests pass locally with my changes

[eigan]

I think perhaps the tests should actually try to generate a QR with this data too. As it will crash in AlphaNum::ord(). Would have revealed this bug earlier.

[eigan]

DataInterfaceTestAbstract

	public function testValidateString(string $string, bool $expected):void{
		$this::assertSame($expected, $this->dataMode::validateString($string));

		try {
			$qrCode      = new QRCode;
			$qrCode->addSegment(static::getDataModeInterface($string));
			$qrCode->renderMatrix($qrCode->getQRMatrix());
			self::assertTrue($expected);
		} catch(\Exception){
			self::assertFalse($expected);
		}
	}

This can also reveal the real bug while in main.

[codemasher]

Oh that's a good one, thank you! I think I should check other regular expressions here too at this point. I couldn't even tell you why I checked the comma for true in the test.

Adding an extra check running the encoder/decoder is a great idea, so I've tried this and gave me bit of a headache:

	public function testValidateString(string $string, bool $expected):void{
		$this::assertSame($expected, $this->dataMode::validateString($string));

		// back out on potentially invalid strings
		if($expected === false){
			return;
		}

		$bitBuffer         = new BitBuffer;
		$dataModeInterface = static::getDataModeInterface($string);
		// check if the vailidated string encodes without error
		$dataModeInterface->write($bitBuffer, 5);
		$bitBuffer->rewind();
		// decode and check against the given string
		$decoded = $dataModeInterface::decodeSegment($bitBuffer, $bitBuffer->read(4));

		$this::assertSame($string, $decoded);
	}

Which throws an error with the Hanzi encoder:

Failed asserting that two strings are identical.
Expected :'原神'
Actual   :'瞅唷　　　　　　　　'

...\tests\Data\DataInterfaceTestAbstract.php:89


Failed asserting that two strings are identical.
Expected :'无可奈何燃花作香'
Actual   :'蠖堵收黯夙满魃　　　　　　　　　　　　　　　　　　　　　　　　　'

...\tests\Data\DataInterfaceTestAbstract.php:89

[codemasher]

Ah nevermind, it was just my brain that is goo currently. the 4 bits i read from the BitBuffer are the data mode, not the version number.

	public function testValidateString(string $string, bool $expected):void{
		$this::assertSame($expected, $this->dataMode::validateString($string));

		// back out on potentially invalid strings
		if($expected === false){
			return;
		}

		$bitBuffer         = new BitBuffer;
		$dataModeInterface = static::getDataModeInterface($string);
		// check if the validated string encodes without error
		$dataModeInterface->write($bitBuffer, 5);
		$bitBuffer->rewind();
		// read 4 bits (data mode indicator)
		$bitBuffer->read(4);
		// decode and check against the given string
		$decoded = $dataModeInterface::decodeSegment($bitBuffer, 5);

		$this::assertSame($string, $decoded);
	}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.31%. Comparing base (24d4e6f) to head (29cdbcc).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main     #313   +/-   ##
=========================================
  Coverage     91.31%   91.31%           
  Complexity     1218     1218           
=========================================
  Files            57       57           
  Lines          3119     3119           
=========================================
  Hits           2848     2848           
  Misses          271      271           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[codemasher]

Thank you for your contribution!

(the recently failed CI was presented by Codacy!)