Skip to content

Various enhancements and fixes from Northwestern's Avalon implementation - #6925

Open
mbklein wants to merge 7 commits into
avalonmediasystem:developfrom
nulib:nul-fixes
Open

Various enhancements and fixes from Northwestern's Avalon implementation#6925
mbklein wants to merge 7 commits into
avalonmediasystem:developfrom
nulib:nul-fixes

Conversation

@mbklein

@mbklein mbklein commented Jul 29, 2026

Copy link
Copy Markdown
Member

This PR contains multiple small fixes we've applied on top of several versions of Avalon while applying our own branding and additional in-house features to the base version. They include:

  • A fix for a situation where poster generation would raise an exception if master_file.file_location was nil (e.g., when the MasterFile management strategy is set to delete)
  • A fix for streaming URLs with spaces in them (e.g., when a MasterFile is ingested with spaces in the filename and goes through the whole pipeline like that). (This might only apply to AWS CloudFront streaming, but shouldn't break any others)
  • A fix to allow the MediaPlayer component to send credentials (cookies and/or auth headers) with streaming requests
  • A CatalogController before_action to validate sort params before allowing the request to continue
  • A fix to prevent HTML encoding of URLs inside dynamically generated HLS manifests
  • An enhancement to allow WaveformJob to generate waveforms from an AWS S3 source by streaming it instead of downloading the entire file to temp storage
  • A fix for batch ingest that makes sure that the ingest object's @current_package instance variable always gets set correctly

I left them as separate commits to make them easier to cherry pick, but I didn't want to submit 7 separate tiny PRs.

@joncameron

Copy link
Copy Markdown
Contributor

We'll review and pull in after 8.2.1 release.

@mbklein

mbklein commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

I just realized that I based this PR against main instead of develop and I can't change it now. I can either close it and recreate it, or someone with admin access to the repo can probably change the base.

@masaball
masaball changed the base branch from main to develop August 3, 2026 13:21
mbklein added a commit to nulib/avalon that referenced this pull request Aug 5, 2026
AVR redirects the player straight at the pre-rendered playlist on
CloudFront (see the next commit), so the URL itself has to carry a
signature -- upstream only needs cookies, because its player always fetches
playlists through Avalon.

Switches from the cloudfront-signer gem to aws-sdk-cloudfront's
Url/CookieSigner: cloudfront-signer configures one process-wide signer via
Aws::CF::Signer.configure that can't be re-keyed, and offers no URL signing.
(cloudfront-signer stays in upstream's Gemfile; removing it would be one
more line of Gemfile conflict for no gain.)

Also fixes the signed-cookie domain, which upstream computes as the set
intersection of the app and streaming hosts' labels. That's order- and
position-blind: for an app host of avr.library.northwestern.edu and a
streaming host of stream.avr.library.northwestern.edu it returns
avr.library.northwestern.edu -- a domain the streaming host is not under, so
the browser drops the cookie. Now walks in from the TLD to find the real
common suffix. Specs added for all three cases.

Reworked while restructuring:

  * Dropped the ':s3' streaming server branch. Settings.streaming.server is
    'aws' in every AVR environment (terraform/settings.tf and .envrc both
    hardcode it), so the branch was unreachable.
  * Dropped a copyright-header revert that reintroduced trailing whitespace.
  * Replaced AVR's URI escape/unescape round-trip for filenames containing
    spaces with the simpler .gsub(' ', '%20') now upstream in PR avalonmediasystem#6925.
mbklein added a commit to nulib/avalon that referenced this pull request Aug 11, 2026
AVR redirects the player straight at the pre-rendered playlist on
CloudFront (see the next commit), so the URL itself has to carry a
signature -- upstream only needs cookies, because its player always fetches
playlists through Avalon.

Switches from the cloudfront-signer gem to aws-sdk-cloudfront's
Url/CookieSigner: cloudfront-signer configures one process-wide signer via
Aws::CF::Signer.configure that can't be re-keyed, and offers no URL signing.
(cloudfront-signer stays in upstream's Gemfile; removing it would be one
more line of Gemfile conflict for no gain.)

Also fixes the signed-cookie domain, which upstream computes as the set
intersection of the app and streaming hosts' labels. That's order- and
position-blind: for an app host of avr.library.northwestern.edu and a
streaming host of stream.avr.library.northwestern.edu it returns
avr.library.northwestern.edu -- a domain the streaming host is not under, so
the browser drops the cookie. Now walks in from the TLD to find the real
common suffix. Specs added for all three cases.

Reworked while restructuring:

  * Dropped the ':s3' streaming server branch. Settings.streaming.server is
    'aws' in every AVR environment (terraform/settings.tf and .envrc both
    hardcode it), so the branch was unreachable.
  * Dropped a copyright-header revert that reintroduced trailing whitespace.
  * Replaced AVR's URI escape/unescape round-trip for filenames containing
    spaces with the simpler .gsub(' ', '%20') now upstream in PR avalonmediasystem#6925.
mbklein added a commit to nulib/avalon that referenced this pull request Aug 11, 2026
AVR redirects the player straight at the pre-rendered playlist on
CloudFront (see the next commit), so the URL itself has to carry a
signature -- upstream only needs cookies, because its player always fetches
playlists through Avalon.

Switches from the cloudfront-signer gem to aws-sdk-cloudfront's
Url/CookieSigner: cloudfront-signer configures one process-wide signer via
Aws::CF::Signer.configure that can't be re-keyed, and offers no URL signing.
(cloudfront-signer stays in upstream's Gemfile; removing it would be one
more line of Gemfile conflict for no gain.)

Also fixes the signed-cookie domain, which upstream computes as the set
intersection of the app and streaming hosts' labels. That's order- and
position-blind: for an app host of avr.library.northwestern.edu and a
streaming host of stream.avr.library.northwestern.edu it returns
avr.library.northwestern.edu -- a domain the streaming host is not under, so
the browser drops the cookie. Now walks in from the TLD to find the real
common suffix. Specs added for all three cases.

Reworked while restructuring:

  * Dropped the ':s3' streaming server branch. Settings.streaming.server is
    'aws' in every AVR environment (terraform/settings.tf and .envrc both
    hardcode it), so the branch was unreachable.
  * Dropped a copyright-header revert that reintroduced trailing whitespace.
  * Replaced AVR's URI escape/unescape round-trip for filenames containing
    spaces with the simpler .gsub(' ', '%20') now upstream in PR avalonmediasystem#6925.
mbklein added a commit to nulib/avalon that referenced this pull request Aug 12, 2026
AVR redirects the player straight at the pre-rendered playlist on
CloudFront (see the next commit), so the URL itself has to carry a
signature -- upstream only needs cookies, because its player always fetches
playlists through Avalon.

Switches from the cloudfront-signer gem to aws-sdk-cloudfront's
Url/CookieSigner: cloudfront-signer configures one process-wide signer via
Aws::CF::Signer.configure that can't be re-keyed, and offers no URL signing.
(cloudfront-signer stays in upstream's Gemfile; removing it would be one
more line of Gemfile conflict for no gain.)

Also fixes the signed-cookie domain, which upstream computes as the set
intersection of the app and streaming hosts' labels. That's order- and
position-blind: for an app host of avr.library.northwestern.edu and a
streaming host of stream.avr.library.northwestern.edu it returns
avr.library.northwestern.edu -- a domain the streaming host is not under, so
the browser drops the cookie. Now walks in from the TLD to find the real
common suffix. Specs added for all three cases.

Reworked while restructuring:

  * Dropped the ':s3' streaming server branch. Settings.streaming.server is
    'aws' in every AVR environment (terraform/settings.tf and .envrc both
    hardcode it), so the branch was unreachable.
  * Dropped a copyright-header revert that reintroduced trailing whitespace.
  * Replaced AVR's URI escape/unescape round-trip for filenames containing
    spaces with the simpler .gsub(' ', '%20') now upstream in PR avalonmediasystem#6925.
mbklein added a commit to nulib/avalon that referenced this pull request Aug 13, 2026
AVR redirects the player straight at the pre-rendered playlist on
CloudFront (see the next commit), so the URL itself has to carry a
signature -- upstream only needs cookies, because its player always fetches
playlists through Avalon.

Switches from the cloudfront-signer gem to aws-sdk-cloudfront's
Url/CookieSigner: cloudfront-signer configures one process-wide signer via
Aws::CF::Signer.configure that can't be re-keyed, and offers no URL signing.
(cloudfront-signer stays in upstream's Gemfile; removing it would be one
more line of Gemfile conflict for no gain.)

Also fixes the signed-cookie domain, which upstream computes as the set
intersection of the app and streaming hosts' labels. That's order- and
position-blind: for an app host of avr.library.northwestern.edu and a
streaming host of stream.avr.library.northwestern.edu it returns
avr.library.northwestern.edu -- a domain the streaming host is not under, so
the browser drops the cookie. Now walks in from the TLD to find the real
common suffix. Specs added for all three cases.

Reworked while restructuring:

  * Dropped the ':s3' streaming server branch. Settings.streaming.server is
    'aws' in every AVR environment (terraform/settings.tf and .envrc both
    hardcode it), so the branch was unreachable.
  * Dropped a copyright-header revert that reintroduced trailing whitespace.
  * Replaced AVR's URI escape/unescape round-trip for filenames containing
    spaces with the simpler .gsub(' ', '%20') now upstream in PR avalonmediasystem#6925.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants