LycheeOrg / Lychee

A great looking and easy-to-use photo-management-system you can run on your server, to manage and share photos.
https://lycheeorg.github.io/
MIT License
3.42k stars 303 forks source link

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

Closed HorlogeSkynet closed 9 months ago

HorlogeSkynet commented 10 months ago

Hello :wave:

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 :pray:

ildyria commented 10 months ago

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

HorlogeSkynet commented 10 months ago

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 :wave:

ildyria commented 10 months ago

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 commented 9 months ago

I think that hacky systems should not be used.

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

ildyria commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

What did I just write ?

haivala commented 9 months ago

What did I just write ?

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

ildyria commented 9 months ago

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

HorlogeSkynet commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

Nope, no more #

d7415 commented 9 months ago

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 commented 9 months ago

Yeah. Every day you learn something. :)

HorlogeSkynet commented 9 months ago

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 commented 9 months ago

@HorlogeSkynet see #2240

ildyria commented 9 months ago

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 commented 9 months ago

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

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 :wave: