Uploaded image path looks incorrect #597

Closed
opened 2026-02-04 21:22:11 +03:00 by OVERLORD · 4 comments
Owner

Originally created by @Abijeet on GitHub (Mar 11, 2018).

For Bug Reports

  • BookStack Version: master branch
  • PHP Version: 7.0.x
  • MySQL Version: MariaDB 10.1.x

Found this while checking why gif aren't playing.

Expected Behavior

Image path should contain Year, month and day.

Current Behavior

Sample URL -
127.0.0.1:8000/uploads/images/gallery/2018-03-Mar/scaled-840-0/anigif_enhanced-buzz-14847-1355769124-2.gif

Image path contains Year, month and Month (Y-m-M)

This code looks suspicious,

$imagePath = '/uploads/images/' . $type . '/' . Date('Y-m-M') . '/';

c44c42103c/app/Services/ImageService.php (L130)

Should this be Y-m-D? I don't think fixing this will cause issues with existing URLs?

Steps to Reproduce

Upload any image, and check the URL.

Originally created by @Abijeet on GitHub (Mar 11, 2018). ### For Bug Reports * BookStack Version: **master branch** * PHP Version: 7.0.x * MySQL Version: MariaDB 10.1.x Found this while checking why gif aren't playing. ### Expected Behavior Image path should contain Year, month and day. ### Current Behavior **Sample URL** - 127.0.0.1:8000/uploads/images/gallery/**2018-03-Mar**/scaled-840-0/anigif_enhanced-buzz-14847-1355769124-2.gif Image path contains Year, month and Month (Y-m-M) This code looks suspicious, ``` $imagePath = '/uploads/images/' . $type . '/' . Date('Y-m-M') . '/'; ``` https://github.com/BookStackApp/BookStack/blob/c44c42103c9e1aa324ce706b4acb8e5076af9610/app/Services/ImageService.php#L130 Should this be `Y-m-D`? I don't think fixing this will cause issues with existing URLs? ### Steps to Reproduce Upload any image, and check the URL.
OVERLORD added the Open to discussion label 2026-02-04 21:22:11 +03:00
Author
Owner

@ssddanbrown commented on GitHub (Mar 12, 2018):

@Abijeet Awesome, I didn't know you could embed source lines like that into an issue, That's going to be super useful in the future!

In regards to this issue, This isn't incorrect but likely just my thinking perhaps being unusual. I like the month name being in there for readability, so when you're quickly scanning over folders you can look for the month name. Unfortunately month names are not in chronological order which is why I've also put month numbers before it.

I generally didn't want to go down to day-level as that would produce a large amount of folders. Looking back on this it is a bit silly, Would be happy to change to 2018-03 format if generally preferred and yeah, Shouldn't cause issues with existing images. Will leave open to discussion to gather further opinions.

@ssddanbrown commented on GitHub (Mar 12, 2018): @Abijeet Awesome, I didn't know you could embed source lines like that into an issue, That's going to be super useful in the future! In regards to this issue, This isn't incorrect but likely just my thinking perhaps being unusual. I like the month name being in there for readability, so when you're quickly scanning over folders you can look for the month name. Unfortunately month names are not in chronological order which is why I've also put month numbers before it. I generally didn't want to go down to day-level as that would produce a large amount of folders. Looking back on this it is a bit silly, Would be happy to change to `2018-03` format if generally preferred and yeah, Shouldn't cause issues with existing images. Will leave open to discussion to gather further opinions.
Author
Owner

@thomasjsn commented on GitHub (Oct 11, 2018):

This would be a very easy thing to add to the environment file, making everyone happy :) I also don't like day levels on these folders, month is enough.

@thomasjsn commented on GitHub (Oct 11, 2018): This would be a very easy thing to add to the environment file, making everyone happy :) I also don't like day levels on these folders, month is enough.
Author
Owner

@thomasjsn commented on GitHub (Oct 13, 2018):

@ssddanbrown You mind if I make this configurable?

$imagePath = '/uploads/images/' . $type . '/' . Date(config('upload_folder_date_format')) . '/'; 

'upload_folder_date_format' = env('UPLOAD_FOLDER_DATE_FORMAT', 'Y-m-M');

In what file does the config entry belong? Also; suggestions for the configuration variable?

@thomasjsn commented on GitHub (Oct 13, 2018): @ssddanbrown You mind if I make this configurable? ```php $imagePath = '/uploads/images/' . $type . '/' . Date(config('upload_folder_date_format')) . '/'; 'upload_folder_date_format' = env('UPLOAD_FOLDER_DATE_FORMAT', 'Y-m-M'); ``` In what file does the config entry belong? Also; suggestions for the configuration variable?
Author
Owner

@ssddanbrown commented on GitHub (May 7, 2019):

Forgot to put this in the release notes, but while refactoring many image endpoints I've updated the folder to just be Y-m for simplicity and to prevent further future confusion, Therefore I'll close this.

@ssddanbrown commented on GitHub (May 7, 2019): Forgot to put this in the release notes, but while refactoring many image endpoints I've updated the folder to just be `Y-m` for simplicity and to prevent further future confusion, Therefore I'll close this.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/BookStack#597