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

[backward comp] v5 broke v4 shared album URLs (using fragments) : provide hook for /gallery#albumID. #2176

Closed
HorlogeSkynet opened this issue Jan 7, 2024 · 20 comments · Fixed by #2227
Labels
Low Priority Low priority issues

Comments

@HorlogeSkynet
Copy link
Contributor

HorlogeSkynet commented Jan 7, 2024

Hello 👋

Detailed description of the problem

All previous (v4) shared album URLs do not work against v5 due to LycheeOrg/Lychee-front#343 rework.
Although I fully-agree with the rationale, was the feature expected to be non backward-compatible ?
Do you think a quick client-side "hook" could be added to restore "pre-v5" URLs compatibility ?

Steps to reproduce the issue

  1. Open a previously shared album URL after Lychee v5 upgrade
  2. Lychee asks for credentials (if logged out), or simply does nothing (when logged in)

Output of the diagnostics

Diagnostics

Diagnostics

Error: Wrong property for recent_age, expected strictly positive integer, got 0.
Warning: Dropbox import not working. dropbox_key is empty.
Error: APP_URL does not match the current url. This will break WebAuthn authentication.
Error: APP_URL does not match the current url. This will prevent images to be properly displayed.
Info: Latest version of PHP is 8.3
 Foreign key: access_permissions.user_id     → users.id            
 Foreign key: albums.cover_id                → photos.id           
 Foreign key: albums.id                      → base_albums.id      
 Foreign key: albums.parent_id               → albums.id           
 Foreign key: base_albums.owner_id           → users.id            
 Foreign key: jobs_history.owner_id          → users.id            
 Foreign key: photos.album_id                → albums.id           
 Foreign key: photos.owner_id                → users.id            
 Foreign key: size_variants.photo_id         → photos.id           
 Foreign key: sym_links.size_variant_id      → size_variants.id    
 Foreign key: tag_albums.id                  → base_albums.id      

System Information

Lychee Version (git):                    ?? (33354a2) -- Could not compare.
DB Version:                              5.0.2

composer install:                        --no-dev
APP_ENV:                                 production
APP_DEBUG:                               false
APP_URL:                                 set

System:                                  Linux
PHP Version:                             8.2.7
PHP User agent:                          Lychee/4 (https://lycheeorg.github.io/)
Timezone:                                Europe/Paris
Max uploaded file size:                  100M
Max post size:                           100M
Livewire chunk size:                     12.00 MB
Max execution time:                      3600
PostgreSQL Version:                      PostgreSQL 15.5 (Debian 15.5-0+deb12u1) on x86_64-pc-linux-gnu, compiled by gcc (Debian 12.2.0-14) 12.2.0, 64-bit

exec() Available:                        yes
Imagick Available:                       1
Imagick Enabled:                         1
Imagick Version:                         1691
GD Version:                              2.3.3
Number of foreign key:                   11 found.

Config Information

version:                                 050002
check_for_updates:                       0
sorting_photos_col:                      taken_at
sorting_photos_order:                    ASC
sorting_albums_col:                      max_taken_at
sorting_albums_order:                    ASC
imagick:                                 1
skip_duplicates:                         0
small_max_width:                         0
small_max_height:                        360
medium_max_width:                        1920
medium_max_height:                       1080
lang:                                    fr
image_overlay_type:                      desc
default_license:                         none
compression_quality:                     90
grants_full_photo_access:                1
delete_imported:                         0
mod_frame_enabled:                       1
mod_frame_refresh:                       30
thumb_2x:                                1
small_2x:                                1
medium_2x:                               1
landing_page_enable:                     0
site_owner:                              
landing_title:                           
landing_subtitle:                        
sm_facebook_url:                         
sm_flickr_url:                           
sm_twitter_url:                          
sm_instagram_url:                        
sm_youtube_url:                          
landing_background:                      dist/cat.webp
site_title:                              Lychee
footer_show_copyright:                   0
site_copyright_begin:                    2017
site_copyright_end:                      2021
footer_additional_text:                  
footer_show_social_media:                0
search_public:                           0
SL_enable:                               0
SL_for_admin:                            0
recent_age:                              0
grants_download:                         0
photos_wraparound:                       1
map_display:                             0
zip64:                                   1
map_display_public:                      0
map_provider:                            Wikimedia
force_32bit_ids:                         0
map_include_subalbums:                   1
update_check_every_days:                 7
has_exiftool:                            1
share_button_visible:                    0
import_via_symlink:                      0
has_ffmpeg:                              1
location_decoding:                       0
location_decoding_timeout:               30
location_show:                           1
location_show_public:                    0
rss_enable:                              0
rss_recent_days:                         7
rss_max_items:                           100
prefer_available_xmp_metadata:           0
editor_enabled:                          1
lossless_optimization:                   0
swipe_tolerance_x:                       150
swipe_tolerance_y:                       250
local_takestamp_video_formats:           .avi|.mov
log_max_num_line:                        1000
unlock_password_photos_with_url_param:   0
nsfw_visible:                            1
nsfw_blur:                               0
nsfw_warning:                            0
nsfw_warning_admin:                      0
nsfw_banner_override:                    
map_display_direction:                   1
album_subtitle_type:                     oldstyle
upload_processing_limit:                 4
new_photos_notification:                 0
legacy_id_redirection:                   1
zip_deflate_level:                       6
SA_enabled:                              0
default_album_protection:                1
allow_username_change:                   1
album_decoration:                        layers
album_decoration_orientation:            row
auto_fix_orientation:                    1
use_job_queues:                          0
random_album_id:                         starred
use_last_modified_date_when_no_exif_date: 0
ffmpeg_path:                             /usr/bin/ffmpeg
ffprobe_path:                            /usr/bin/ffprobe
layout:                                  justified
date_format_photo_thumb:                 M j, Y, g:i:s A e
date_format_photo_overlay:               M j, Y, g:i:s A e
date_format_sidebar_uploaded:            M j, Y, g:i:s A e
date_format_sidebar_taken_at:            M j, Y, g:i:s A e
date_format_hero_min_max:                F Y
date_format_hero_created_at:             M j, Y, g:i:s A T
date_format_album_thumb:                 M Y
upload_chunk_size:                       0
nsfw_banner_blur_backdrop:               0
search_pagination_limit:                 1000
search_minimum_length_required:          4
photo_layout_justified_row_height:       320
photo_layout_masonry_column_width:       300
photo_layout_grid_column_width:          250
photo_layout_square_column_width:        200
photo_layout_gap:                        12
display_thumb_album_overlay:             always
display_thumb_photo_overlay:             hover

Browser and system

Tested on Firefox (if relevant).


Thanks ! Bye 🙏

@ildyria
Copy link
Member

ildyria commented Jan 7, 2024

It is slightly trickier than that.

The first reason is that now we automatically redirect to /landing or /gallery if you are arriving on the tld.
There is no way to detect server side if there is a fragment unless we pretty much create another layer of JS execution + redirection.

The change v4~5 is indeed backward incompatible with that kind of sharing. However if you shared using example.com/s/albumID then we do take care of proper forwarding.

v5 is a Major version (hence backward compatibility breaking change), indeed, this breaking change should be documented.

What could be done however is to have a JS hook on fragments on the album page if you are were already sharing with domain.tld/gallery. That should be doable (but honestly low prio).

Code to be changed is most likely to be found here:
https://github.com/LycheeOrg/Lychee/blob/master/resources/js/app.ts
or here:
https://github.com/LycheeOrg/Lychee/blob/master/resources/js/data/views/albumView.ts

@ildyria ildyria added the Low Priority Low priority issues label Jan 7, 2024
@ildyria ildyria changed the title v5 broke v4 shared album URLs [backward comp] v5 broke v4 shared album URLs (using fragments) : provide hook for /gallery#albumID. Jan 7, 2024
@HorlogeSkynet
Copy link
Contributor Author

HorlogeSkynet commented Jan 7, 2024

Thanks for your quick answer. I noticed most of URLs I've shared look like domain.tld/#$albumID. Do you think such a "hackish-hook" should handle these as well ?

Bye 👋

@ildyria
Copy link
Member

ildyria commented Jan 7, 2024

I noticed most of URLs I've shared look like domain.tld/#$albumID. Do you think such a "hackish-hook" should handle these as well ?

That is pretty much the hard case. :/
This would need to add a step here: https://github.com/LycheeOrg/Lychee/blob/master/routes/web-livewire.php#L39 and quite a bit of code. :(
I can give it a try but no guarantee when it would be available.

@haivala
Copy link

haivala commented Jan 22, 2024

I think that hacky systems should not be used.

How about redirect rules for the web server? I only need the album one.

@ildyria
Copy link
Member

ildyria commented Jan 22, 2024

It works both for album and photo.

If you were using https://lychee.test/r/albumId/{photoId} those have already been taken care of at initial release. :)

Only thing not working is when it was shared as https://lychee.test/gallery#albumId/{photoId}

@haivala
Copy link

haivala commented Jan 22, 2024

I just now tried and it does not work with /gallery#OO1G5ahoUqecUhHTlOZ_VrSD
It opens front page and yes I have set the env variable.

@ildyria
Copy link
Member

ildyria commented Jan 22, 2024

What did I just write ?

@haivala
Copy link

haivala commented Jan 22, 2024

What did I just write ?

And I was suggesting that these should be web server redirect rules rather than in the code.

@ildyria
Copy link
Member

ildyria commented Jan 22, 2024

Not possible, the web-server does not have access to the url fragment.

@HorlogeSkynet
Copy link
Contributor Author

I think that hacky systems should not be used.

That's why it is behind an environment toggle.


Many thanks @ildyria, can't wait to give it a try.

@haivala
Copy link

haivala commented Jan 22, 2024

Not possible, the web-server does not have access to the url fragment.

ah. Well ok then. I thought the fragment would be the same because I tested to change old link /gallery#OO1G5ahoUqecUhHTlOZ_VrSD to /gallery/OO1G5ahoUqecUhHTlOZ_VrSD and it worked.

@ildyria
Copy link
Member

ildyria commented Jan 22, 2024

ah. Well ok then. I thought the fragment would be the same because I tested to change old link /gallery#OO1G5ahoUqecUhHTlOZ_VrSD to /gallery/OO1G5ahoUqecUhHTlOZ_VrSD and it worked.

I would have done so otherwise :')

We need to add the small hacky bit of code in the gallery page too at some point to take care of that missing redirection.

@haivala
Copy link

haivala commented Jan 22, 2024

I have to ask: Does the system use # in urls anymore at all?
I was thinking to test to rewrite urls to substitute # with /

@ildyria
Copy link
Member

ildyria commented Jan 22, 2024

Nope, no more #

@d7415
Copy link
Contributor

d7415 commented Jan 22, 2024

I have to ask: Does the system use # in urls anymore at all? I was thinking to test to rewrite urls to substitute # with /

If you mean on the httpd side, note that it won't be seeing the fragments.

@haivala
Copy link

haivala commented Jan 22, 2024

Yeah. Every day you learn something. :)

@HorlogeSkynet
Copy link
Contributor Author

Note : the workaround does not work (in my case) as it violates CSP. I tried a quick-and-dirty patch with script-src nonce, in vain.

@ildyria
Copy link
Member

ildyria commented Jan 23, 2024

@HorlogeSkynet see #2240

@ildyria
Copy link
Member

ildyria commented Jan 23, 2024

If you want to try the patch: patch-diff.githubusercontent.com/raw/LycheeOrg/Lychee/pull/2241.patch

Do note that it will also push you to candidate 5.1.2 (but that can be easily reverted by doing a php artisan migrate:rollback and updating the value in version.md.

Once the PR is merged we will release 5.1.2 fixing this.
Waiting for the colleagues to review.

@HorlogeSkynet
Copy link
Contributor Author

Hey @ildyria, I just wanted to tell you that the patch is working like a charm 👌

I've coupled it with a simple Apache mod_rewrite usage to extend support for <= v3 links too :

RewriteEngine On
RewriteMap v3_links "txt:/etc/apache2/sites-available/lychee_v3_links.txt"
RewriteRule "^/gallery/([0-9]+)$" "/gallery/${v3_links:$1}" [END,NE,R=permanent]

... where lychee_v3_links.txt contains a simple mapping as below :

$old_lychee_album_id_1 $new_lychee_album_id_AAA
$old_lychee_album_id_2 $new_lychee_album_id_BBB
# ...

Thanks, bye 👋

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Low Priority Low priority issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants