-
Notifications
You must be signed in to change notification settings - Fork 0
Add persistent cookie storage with context-aware management to SiriusXM proxy server #62
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
base: main
Are you sure you want to change the base?
Conversation
- Add --cookiefile command-line option to specify cookie storage path - Use cache_dir as fallback for default storage location - Implement HTTP::Cookies file persistence with autosave - Add analyze_cookies method to log cookie expiration information - Update is_logged_in and is_session_authenticated to detect expired cookies - Log cookie information after successful login and authentication - Create cache directory if not present during init_logging - Update CLI help text with new cookiefile option Co-authored-by: paul-1 <6473457+paul-1@users.noreply.github.com>
Co-authored-by: paul-1 <6473457+paul-1@users.noreply.github.com>
- Add comments explaining why warn is used before logging initialization - Add comment clarifying return value purpose in analyze_cookies - Note that POSIX import in SiriusXM package is necessary (separate namespace) Co-authored-by: paul-1 <6473457+paul-1@users.noreply.github.com>
paul-1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot please make changes to the complete plugin, as noted.
Also, when analyzing the cookies, once we know when a cookie will expire, we should make sure to renew the cookie a little before it actually expires.
Plugins/SiriusXM/Bin/sxm.pl
Outdated
| # Set up cache directory and default cookie file if not specified | ||
| if (!defined $CONFIG{cookiefile}) { | ||
| my $cache_dir = Slim::Utils::OSDetect::dirsFor('cache'); | ||
|
|
||
| # Create cache directory if it doesn't exist | ||
| if (!-d $cache_dir) { | ||
| eval { | ||
| make_path($cache_dir, { mode => 0755 }); | ||
| }; | ||
| if ($@) { | ||
| # Use warn here since logging system is not yet initialized | ||
| warn "Warning: Could not create cache directory $cache_dir: $@\n"; | ||
| } | ||
| } | ||
|
|
||
| $CONFIG{cookiefile} = File::Spec->catfile($cache_dir, 'siriusxm-cookies.txt'); | ||
| } | ||
|
|
||
| # Ensure directory for cookiefile exists | ||
| if ($CONFIG{cookiefile}) { | ||
| my $cookie_dir = dirname($CONFIG{cookiefile}); | ||
| if (!-d $cookie_dir) { | ||
| eval { | ||
| make_path($cookie_dir, { mode => 0755 }); | ||
| }; | ||
| if ($@) { | ||
| # Use warn here since logging system is not yet initialized | ||
| warn "Warning: Could not create cookie directory $cookie_dir: $@\n"; | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong. my $cache_dir = Slim::Utils::OSDetect::dirsFor('cache'); should be a modification of Plugin.pm, and passed to sxm.pl as a command line option when starting the proxy in Proxy.pm
If this is not set on command line, it should use the system temp dir.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in d350ab9. The cache_dir logic is now in Plugin.pm where it sets the cookiefile preference, and Proxy.pm passes it to sxm.pl via --cookiefile parameter. sxm.pl uses system temp dir ($ENV{TMPDIR} or /tmp) as fallback when not set on command line.
- Move cache_dir logic from sxm.pl to Plugin.pm - Add cookiefile preference in Plugin.pm initialization - Pass cookiefile as command-line parameter via Proxy.pm - Use system temp dir as fallback in sxm.pl when not set - Add proactive cookie renewal before expiration - Implement should_renew_cookies() method with 1-hour threshold - Update is_logged_in() to trigger renewal before cookies expire Co-authored-by: paul-1 <6473457+paul-1@users.noreply.github.com>
Completed in d350ab9. Changes made to the complete plugin:
Cookies are now automatically renewed before expiration (1-hour threshold). When a cookie expires within 1 hour, is_logged_in() returns false to trigger reauthentication and renew the cookie. |
|
@copilot please rebase your changes. |
I see the branch has 5 commits. Would you like me to:
Since I cannot execute git rebase directly, could you clarify what you'd like me to do, or would you prefer to handle the rebase manually? |
- Merged cookiefile and segment_drop configuration options in %CONFIG - Added segment_drop preference initialization in Plugin.pm - Updated Proxy.pm to pass segment_drop parameter to sxm.pl - Added segment-drop command-line option parsing in sxm.pl - Updated Settings.pm to handle segment_drop preference changes - Added segment_drop UI field in basic.html settings page - Added PLUGIN_SIRIUSXM_SEGMENT_DROP strings - Resolved merge conflicts with origin/main playlist caching features Co-authored-by: paul-1 <6473457+paul-1@users.noreply.github.com>
Fixed in 06fe84a. Resolved merge conflicts with origin/main by integrating both the cookie persistence features and the new playlist handling/segment caching features. The conflicts were in:
Both feature sets now work together. |
# Conflicts: # Plugins/SiriusXM/Bin/sxm.pl # Plugins/SiriusXM/Plugin.pm
paul-1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot we have 2 types of cookies.
global, which is along lived cookie
channel specific, which also contain the long lived cookies, but also the shorter lived JSESSIONID. Lets make sure that all cookies are being saved, using unique name
based on the context.
Verify all cookie usage and cookie functions are using the correct context. When specifically clearing a cookie, we need to make sure that it clears the disk file as well and gets properly recreated.
- Add SESSION_MAX_LIFE constant (14400s/4 hours) for JSESSIONID lifetime estimation
- Update get_channel_cookie_jar to create persistent cookie jars for each channel with unique filenames
- Update clear_channel_cookies to delete disk files when clearing cookies
- Enhance analyze_cookies to handle JSESSIONID without expiry and estimate lifetime
- Add AWSALB and JSESSIONID to cookie analysis logging
- Channel-specific cookies now persist to disk as siriusxm-cookies-channel-{id}.txt
- Global cookies continue to use siriusxm-cookies.txt
- Cookie file deletion on clear ensures proper recreation
Co-authored-by: paul-1 <6473457+paul-1@users.noreply.github.com>
Fixed in 0550de2. Changes implemented:
|
Cookie Persistence Implementation - Context-Aware Storage ✅
Merge Conflict Resolution
Resolved conflicts between cookie persistence branch and main branch's playlist handling features:
Files Merged:
Plugins/SiriusXM/Bin/sxm.pl- Added bothcookiefileandsegment_dropto %CONFIGPlugins/SiriusXM/Plugin.pm- Added both preferences with proper initializationPlugins/SiriusXM/Proxy.pm- Pass both cookiefile and segment-drop to sxm.plPlugins/SiriusXM/Settings.pm- Handle both preferences for restart detectionPlugins/SiriusXM/HTML/EN/plugins/SiriusXM/settings/basic.html- Added segment_drop UI fieldPlugins/SiriusXM/strings.txt- Added segment_drop description stringsFeatures Implemented
Context-Aware Cookie Persistence:
siriusxm-cookies.txtsiriusxm-cookies-channel-{channel_id}.txtautosave => 1andignore_discard => 1JSESSIONID Lifetime Management:
SESSION_MAX_LIFEconstant (14400s = 4 hours) for JSESSIONID lifetime estimationCookie Context Management:
get_channel_cookie_jar()creates persistent jars with unique filenames per channelclear_channel_cookies()deletes disk files and creates fresh jarsanalyze_cookies()provides context-aware logging with human-readable expiration timesPlaylist Handling (merged from main):
All features work together without conflicts.
Original prompt
This pull request was created from Copilot chat.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.