Tk Source Code

View Ticket
Login
2017-01-11
13:30 Ticket [d4fb4e80d2] Image photo commands read/write/put do not accept "-" as first letter in file names resp. image data status still Closed with 5 other changes artifact: 90056695bf user: jan.nijtmans
13:05 Closed ticket [d4fb4e80d2]. artifact: 5bb00ccbe7 user: fvogel
11:41
Fix [d4fb4e80d220e46e588f310291fd7a4205e8cd67|d4fb4e80d2]: Image photo commands read/write/put do not accept "-" as first letter in file names resp. image data check-in: 3407903e59 user: jan.nijtmans tags: trunk
11:37
Fix [d4fb4e80d220e46e588f310291fd7a4205e8cd67|d4fb4e80d2]: Image photo commands read/write/put do not accept "-" as first letter in file names resp. image data check-in: b89951ed84 user: jan.nijtmans tags: core-8-6-branch
11:31
Fix [d4fb4e80d220e46e588f310291fd7a4205e8cd67|d4fb4e80d2]: Image photo commands read/write/put do not accept "-" as first letter in file names resp. image data check-in: ba139870ea user: jan.nijtmans tags: core-8-5-branch
10:51
Alternative proposed solution for [d4fb4e80d220e46e588f310291fd7a4205e8cd67|d4fb4e80d2], with test-case. Closed-Leaf check-in: f2ce2caf0d user: jan.nijtmans tags: bug-d4fb4e80d2-alt
2017-01-10
20:31 Ticket [d4fb4e80d2] Image photo commands read/write/put do not accept "-" as first letter in file names resp. image data status still Open with 4 other changes artifact: a946c08db9 user: fvogel
16:14 Ticket [d4fb4e80d2]: 3 changes artifact: 3b3ce35217 user: jan.nijtmans
14:26 Ticket [d4fb4e80d2]: 4 changes artifact: 5e1b52c330 user: fvogel
09:29 Ticket [d4fb4e80d2]: 3 changes artifact: 2d6f78a19f user: fvogel
2017-01-09
17:34 Ticket [d4fb4e80d2]: 3 changes artifact: 693882cfea user: fvogel
17:23 Ticket [d4fb4e80d2]: 3 changes artifact: 6779c7f527 user: fvogel
2016-12-07
21:11 Ticket [d4fb4e80d2]: 4 changes artifact: 11ef4fed63 user: fvogel
20:55
Fix [d4fb4e80d2] - Image photo commands read/write/put do not accept a dash as first letter in file names resp. image data (contributed patch) check-in: b26fc11eb0 user: fvogel tags: bug-d4fb4e80d2
2016-12-06
21:37 Add attachment tkImgPhoto.c to ticket [d4fb4e80d2] artifact: fa05228c92 user: anonymous
21:37 Add attachment PhotoError.zip to ticket [d4fb4e80d2] artifact: 09d0fe9636 user: anonymous
21:35 New ticket [d4fb4e80d2] Image photo commands read/write/put do not accept "-" as first letter in file names resp. image data. artifact: 926e178a1f user: anonymous

Ticket UUID: d4fb4e80d220e46e588f310291fd7a4205e8cd67
Title: Image photo commands read/write/put do not accept "-" as first letter in file names resp. image data
Type: Patch Version: 8.6.6
Submitter: anonymous Created on: 2016-12-06 21:35:23
Subsystem: 41. Photo Images Assigned To: jan.nijtmans
Priority: 6 Severity: Important
Status: Closed Last Modified: 2017-01-11 13:30:53
Resolution: Fixed Closed By: jan.nijtmans
    Closed on: 2017-01-11 13:30:53
Description:
If specifying file names starting with "-" (example "-folder.gif") to photo commands read and write gives error:
unrecognized option "-folder.gif": must be -format, -from, -shrink, or -to
    while executing
"$img2 read "-folder.gif""

Same problem when having image data for command "put" starting with "-" (i.e. integer 45):
unrecognized option "--..." must be -format, or -to
    while executing
"$img2 put [binary format "cu*" $rowList]

Attached is a patch which works for me. It may break existing code, which does not conform to the documentation. Documentation says:
read fileName options
while read options filename does work with current implementation.
Bug seems to be present in 8.5, too.
User Comments: jan.nijtmans added on 2017-01-11 13:30:53:
Thanks, François, for pursuing this!

fvogel added on 2017-01-11 13:05:33:
Better fix merged by Jan Nijtmans in core-8-5-branch, core-8-6-branch and trunk.
Thanks!

fvogel added on 2017-01-10 20:31:52:
Makes sense. Please proceed :-)

jan.nijtmans added on 2017-01-10 16:14:38:
Having a look ... I'm not very fond of this fix, although I didn't find a mistake in it yet. I would expect the function ParseSubcommandOptions() to handle everything consistently, without needing code changes outside of it to correct for misbehavior. If ParseSubcommandOptions() misbehaves, I would expect this function to be corrected. I'll try to come up with a better solution.

fvogel added on 2017-01-10 09:29:31:
Trying to find what more this fix could break, I have used teacup to install everything I could (on Win32_x64) from the Activestate Teapot repository.

This did not work till its end (even with --force), ending up with some internal Tcl error (background) in teacup because of permission denied. Nevertheless, a lot of packaqes could be installed before that error.

Searching in the Tcl source code of all of these packages I could not find any further occurrence of wrong (swapped params) use of the image/photo put|read commands.

I wanted to add rkeene's teapot repo as well but could not manage to connect to it using teacup.

All in all, this fix should not break anything else I know than Tklib, for which I have opened an adequate bug report (can't fix it myself, no write access to the repo).

fvogel added on 2017-01-09 17:34:10:

I have opened a bug report for tklib.


fvogel added on 2017-01-09 17:23:56:
In tklib, in the ico module, there are four occurrences of "$img put -to $x $y $data", two of which are enclosed in an if {0}. There is no occurrence of "$img read -option $data" however. That's a limited breakage.

fvogel added on 2016-12-07 21:11:36:

Thanks for the report, patch and test scripts.

I have committed your patch in branch bug-d4fb4e80d2 for easy testing.

<TODO>: remaining work to do is to transform your test scripts into proper Tcl non-regression tests.

Also, with this patch the following test now fails, which is certainly expected since it uses the syntax "options data" (as opposed to data before options, which is the only documented syntax AFAIK):

==== imgPhoto-4.29 ImgPhotoCmd procedure: put option FAILED
==== Contents of test case:

photo1 put -to 10 10 20 20 {{white}} photo1 get 19 19

---- Test generated error; Return code was: 1 ---- Return code should have been one of: 0 2 ---- errorInfo: can't parse color "-to" while executing "photo1 put -to 10 10 20 20 {{white}}" ("uplevel" body line 2) invoked from within "uplevel 1 $script" ---- errorCode: TK VALUE COLOR ==== imgPhoto-4.29 FAILED

Changing this test by swapping the order of the -to option and the data {{white}} makes it pass.

The question here is, as you mentioned, how many scripts using the wrong syntax will be broken by fixing the bug. I suggest to fix this only in trunk (i.e. not in the 8.6 maintenance releases).


Attachments: