Skip to content

Download transcoded files only from GoDAM CDN hosts, and read the callback API key from the body - #2248

Open
subodhr258 wants to merge 1 commit into
developfrom
fix/transcoder-callback-files
Open

subodhr258 wants to merge 1 commit into
developfrom
fix/transcoder-callback-files

Conversation

@subodhr258

Copy link
Copy Markdown
Collaborator

What changes

The transcoder callback (the route GoDAM Central calls when a job finishes) now downloads files only from GoDAM's CDN, and reads the API key only from the request body.

Where Change
inc/helpers/custom-functions.php new rtgodam_get_transcoder_file_hosts() (default gdcdn.us, matched with its subdomains; filter rtgodam_transcoder_file_hosts) and rtgodam_is_transcoder_file_url() (https, no user or password, host in the list)
Converted files (files list), admin/class-rtgodam-transcoder-handler.php only .mp4 / .mp3 from a CDN host; fetched with wp_safe_remote_get(); stored only when the content matches the type (wp_check_filetype_and_ext()). A skipped file is logged
MP4 that replaces an upload (mp4_url), inc/classes/class-media-library-ajax.php CDN host only; stored under a name made from the attachment's own file name with .mp4, not from the URL; must be video/mp4; only the attachment's own file is overwritten, another file with the same name is kept and the MP4 gets the next free name
Callback key check, admin/class-rtgodam-transcoder-rest-routes.php the key is read from the request body (form or JSON), as Central sends it, and compared with hash_equals()

Where Central's files come from: every transcoded MP4, MP3, HLS, DASH and thumbnail URL Central builds is https://{account}.gdcdn.us/{job}/{file} (Central's CDN setup in godam-core, plus every URL in the plugin's code, tests and our sites). Central sends the API key in the form body of each callback; the callback URL it is given carries no key.

Testing

Throwaway WordPress (Playground, PHP 8.3 and 7.4) with the 2.5.0 release plus these files; a made-up key; outbound requests blocked except a local server standing in for the CDN.

Check Result
Video callback with DASH, HLS, thumbnails and the MP4 swap works as before (MP4 upload replaced in place; a .mov upload gets its .mp4)
Audio callback replacing the upload with the MP3 works as before
Transcription callback (form body and JSON body) works as before
Failure callback, preflight probe without a key unchanged
A file URL on any other host, or a non-https URL not requested, logged and skipped
A file on the CDN host whose content isn't the expected type refused
mp4_url naming a different file stored under the attachment's own name; the other file is untouched
Key sent only in the query string refused; key in the body works
A CDN host added with the filter works
  • PHPUnit: 95 new tests (TranscoderFileUrlTest, TranscoderCallbackPermissionTest, TranscodedFileDownloadTest, Mp4ReplacementTest); TranscodedAudioMimeTypeTest updated to use a CDN host. Full suite OK (708 tests).
  • phpcs clean; bin/hook-check checks pass.

Worth knowing

  • Sites on a Central whose CDN isn't under gdcdn.us (self-hosted or some staging setups) lose the MP4 and MP3 swap until they add their host:
    add_filter( 'rtgodam_transcoder_file_hosts', function ( $hosts ) { $hosts[] = 'cdn.example.com'; return $hosts; } );
    Only a log line says so. QA on preprod should confirm the MP4 swap still works (step 1 below).
  • An API key in the callback's query string no longer works (Central doesn't send one).
  • A files entry that isn't .mp3 / .mp4 is skipped.
  • Any {account}.gdcdn.us host is accepted, not only the site's own account.

QA:

  1. On a site pointing at preprod and one pointing at production, upload an .mov or .webm video: it transcodes, the MP4 replaces the upload, and HLS/DASH and thumbnails show.
  2. Upload an audio file: it transcodes and the MP3 replaces it.
  3. Run a transcription: the transcript arrives.

Issue: rtCamp/godam-plugin-wp#74

…lback API key from the body

The transcoder callback now downloads files only from GoDAM's CDN hosts:

- Converted MP4 and MP3 files (`files` list) and the MP4 that replaces an
  uploaded video (`mp4_url`) must be https URLs on gdcdn.us or one of its
  subdomains, which is where GoDAM Central serves every account's files.
  Sites on a self-hosted or staging Central can add their CDN domain with
  the new `rtgodam_transcoder_file_hosts` filter. A skipped file is logged.
- Converted files are fetched with wp_safe_remote_get() and are stored only
  when they are an MP4 or MP3 by name and by content.
- The MP4 that replaces an upload is stored under a name made from the
  attachment's own file name with an .mp4 extension, not from the URL. It
  must be video/mp4 (wp_check_filetype_and_ext()), and only the attachment's
  own file is ever overwritten; another file with the same name is kept and
  the MP4 gets the next free name.

Both callback routes now read the API key from the request body (form or
JSON), as GoDAM Central sends it, instead of from any request source, and
compare it with hash_equals().

Adds unit tests for the host check, the key check, the converted file
download and the MP4 replacement, and points the existing converted-audio
test at a GoDAM CDN host.
@subodhr258 subodhr258 self-assigned this Oct 23, 2026
@github-actions

Copy link
Copy Markdown

🔍 WordPress Plugin Check Report

⚠️ Status: Passed with warnings

📊 Report

🎯 Total Issues ❌ Errors ⚠️ Warnings
13 0 13

⚠️ Warnings (13)

📁 readme.txt (2 warnings)
📍 Line 🔖 Check 💬 Message
0 mismatched_plugin_name Plugin name "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" is different from the name declared in plugin header "GoDAM".
0 trademarked_term The plugin name includes a restricted term. Your chosen plugin name - "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" - contains the restricted term "wordpress" which cannot be used at all in your plugin name.
📁 assets/build/css/main.css (1 warning)
📍 Line 🔖 Check 💬 Message
0 EnqueuedStylesScope This style is being loaded in all contexts.
📁 assets/src/libs/analytics.min.js (5 warnings)
📍 Line 🔖 Check 💬 Message
0 EnqueuedScriptsScope This script is being loaded in all frontend contexts.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/2026/10/23/hello-world/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/sample-page/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/demo-attachment-post/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
📁 assets/build/js/main.min.js (5 warnings)
📍 Line 🔖 Check 💬 Message
0 EnqueuedScriptsScope This script is being loaded in all frontend contexts.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/2026/10/23/hello-world/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/sample-page/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/demo-attachment-post/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

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.

1 participant