Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All PsdBooleans are True #10

Closed
widdowson opened this issue Feb 22, 2024 · 4 comments
Closed

All PsdBooleans are True #10

widdowson opened this issue Feb 22, 2024 · 4 comments

Comments

@widdowson
Copy link

Hi there Christoph,

psdtags is great! But I just found a bug while doing an overly[*] paranoid assert(originalbytes == frombytes(originalbytes).tobytes() at the PsdLayer level). It seemed to me that a layer's infx value should have been 0000, but it was being written out as 1000. The same thing was true of knko. But I noticed that there wasn't an unexpected diff in the layer's clbl. They're all PsdBooleans. Then it occurred to me.. it's because they were all hardcoded by psdtags to 1000 (True), and only the clbl was originally true. Lo and behold:

In https://github.com/cgohlke/psdtags/blob/v2024.1.15/psdtags/psdtags.py#L2490 we see:

        value = bool(fh.read(1))

However:

% python3
>>> bool(b'\x01')
True
>>> bool(b'\x00')
True

I believe this is is making all PsdBoolean's read()/frombytes() True, regardless of input value. This also isn't the only place I see a bool(fh.read(1)), which I think is generally a bug for the above reason - unless the read literally returns False or None, you're always going to be True.

I tried patching this in my local checkout to check for fh.read(1) != '\x00', but it had really bizarre side effects - namely that write()/tobytes() output of my PsdLayer object ballooned in size. If I just hardcoded value = True I had identical broken output, but if I got it to parse correctly, I think some other routine is conditioning on some PsdBoolean somewhere, and since it's never been False before (or at least recently - I didn't check blame history) I'm guessing said codepath wasn't exercised.

Hope this helps,
-Andrew

[*] turns out maybe this wasn't overly paranoid, but "just right paranoid" ;)

@cgohlke
Copy link
Owner

cgohlke commented Feb 22, 2024

Thanks for reporting. bool(fh.read(1)) is definitely wrong, should be bool(fh.read(1)[0]).

I rely on the assert to catch cases like this. So far all the files on my system passed with that bug. I certainly don't have enough files for testing and don't have easy access to Photoshop to create new files...

@cgohlke
Copy link
Owner

cgohlke commented Feb 22, 2024

I can't seem to reproduce the bizarre side effect, ballooning. Can you share a file and/or code?

@widdowson
Copy link
Author

Happy to help report bugs, always :). I think there's something going on with my use of channels in PsdLayers that might be causing the problem, and I don't want to waste your time on that since I'm now more convinced it's a glitch on my side. Whoops!

@widdowson widdowson changed the title All PsdBooleans are True - and fix seems to do weird things to write() All PsdBooleans are True Feb 22, 2024
@cgohlke
Copy link
Owner

cgohlke commented Feb 22, 2024

The PsdBoolean read issue should be fixed in v2024.2.22.

@cgohlke cgohlke closed this as completed Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants