Bacon / BaconQrCode

QR Code Generator for PHP
BSD 2-Clause "Simplified" License
1.83k stars 208 forks source link

Fix Shift-JIS throwing Unsupported operand Error #122

Closed askdkc closed 1 year ago

askdkc commented 1 year ago

This will fix private static function isOnlyDoubleByteKanji(string $content) : bool throwing "Unsupported operand types: string & int" when using SHIFT-JIS encoding.

image
DASPRiD commented 1 year ago

Concatenation is definitely incorrect here. Looking at the original Java code it did the bitwise operation, but since it's always a single byte, the correct solution would probably be $byte = ord($bytes[$i]);. This would require some testing though, as this is currently not covered by unit tests (any fix should add unit tests for that).

askdkc commented 1 year ago

@DASPRiD OK, thanks for your comment.

I fixed the code and added test for isOnlyDoubleByteKanji().

Everybody is using UTF-8 these days, and if you are scanning QRcode with your smart phone, it works fine. But cheap QRcode Readers selling in Japan only support SHIFT-JIS encoded QRcode and Windows PC system still uses SHIFT-JIS encoding for Japanese.

BaconQrCode Ver 1 is working fine, but ver 2 having this issue prevents upgrading from ver 1. This problem makes some system unable to upgrade not only BaconQrCode but php itself, that's not good.

Hopefully this fix gets merged and makes Japanese people happy🇯🇵🗻

codecov-commenter commented 1 year ago

Codecov Report

Merging #122 (e8392ae) into master (8674e51) will increase coverage by 0.39%. The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #122      +/-   ##
============================================
+ Coverage     62.88%   63.28%   +0.39%     
  Complexity      934      934              
============================================
  Files            47       47              
  Lines          2727     3086     +359     
============================================
+ Hits           1715     1953     +238     
- Misses         1012     1133     +121     
Impacted Files Coverage Δ
src/Encoder/Encoder.php 90.07% <100.00%> (+3.53%) :arrow_up:
src/Renderer/RendererStyle/Fill.php 68.25% <0.00%> (-9.80%) :arrow_down:
src/Renderer/Color/Rgb.php 41.93% <0.00%> (-8.07%) :arrow_down:
src/Renderer/Image/ImagickImageBackEnd.php 58.88% <0.00%> (-4.27%) :arrow_down:
src/Common/FormatInformation.php 86.79% <0.00%> (-2.34%) :arrow_down:
src/Common/ReedSolomonCodec.php 95.28% <0.00%> (-0.85%) :arrow_down:
src/Renderer/Color/Cmyk.php 0.00% <0.00%> (ø)
src/Renderer/Path/Curve.php 0.00% <0.00%> (ø)
src/Renderer/Eye/SquareEye.php 100.00% <0.00%> (ø)
src/Renderer/Path/EllipticArc.php 0.00% <0.00%> (ø)
... and 12 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

askdkc commented 1 year ago

@DASPRiD

I saw some Lint errors in CI, so I fixed them. Hopefully this PR gets merged and soon to be able to use SHIFT-JIS encoded QRcode again😃

askdkc commented 1 year ago

Hmm, after I apply this fix to the package, appendKanjiBytes(string $content, BitArray $bits) starts throwing an error.

When I generate QRcode with Kanji and ASCII mix characters, it works fine. For example "あいうえお123" can correctly generate QRcode.

But if I try to generate QRcode that contains only double byte Kanji like "日本" it throws this error.

Is there something wrong with appendKanjiBytes(string $content, BitArray $bits) function🤔

image
askdkc commented 1 year ago

This PR was kinda recreated to #124