Modify

Opened 10 years ago

Closed 10 years ago

#1876 closed defect (fixed)

Incorrect image path handling

Reported by: memfis Owned by: Radek Bartoň
Priority: normal Component: ScreenshotsPlugin
Severity: blocker Keywords:
Cc: Trac Release: 0.10

Description

The plugin uses the original filename (including path information) of the uploaded image to store it in the screenshots directory. This causes a problem when uploading from Windows to Linux because the plugin does not seem to handle the path correctly when executing ImageMagick. The effect is that the original file is viewable but none of the resized images are. The filenames created (for the file C:\Windows\Web\Wallpaper\Dom.jpg) are as follows:

/path/to/trac/screenshots# find
.
./1
./1/C:\WINDOWS\Web\Wallpaper\Dom_large.jpg
./1/\WINDOWS\Web\Wallpaper\Dom_medium.jpg
./1/\WINDOWS\Web\Wallpaper\Dom_small.jpg

And this is what the url http://path/to/trac/screenshots/1/medium shows:

Not Found

File /var/lib/trac/test/screenshots/1/C:\WINDOWS\Web\Wallpaper\Dom_medium.jpg not found

Installed components:

  • Trac 0.10.3
  • TracScreenshots 0.4

Attachments (0)

Change History (4)

comment:1 Changed 10 years ago by Radek Bartoň

Status: newassigned

Please try changeset 2542, it should fix this.

comment:2 Changed 10 years ago by memfis

From what I see nothing changed in the way filenames are handled. If I may ask, since every screenshot gets its own id and directory anyway, why not just ignore the filename entirely and just name files "large.jpg", "medium.jpg" and "small.jpg" or with their original name without the path? What purpose does it serve?

comment:3 Changed 10 years ago by Radek Bartoň

I've added to code that gets file from request stripping of path. So instead C:\WINDOWS\Web\Wallpaper\Dom_medium.jpg plugin should get Dom_medium.jpg. If this not work, please send me here part of debug log where is printed recieved file name.

It should behave like you described and on Linux it does but I can't test it with Windows. Dropping filename entirely isn't bad idea and I'll do it like this in the future.

comment:4 Changed 10 years ago by memfis

Resolution: fixed
Status: assignedclosed

Well, I don't know want changed on my server overnight, but it works now just as you described. Maybe it was some code caching issue that prevented the fix from running. Thanks for fixing this.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Radek Bartoň.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.