-
Notifications
You must be signed in to change notification settings - Fork 457
TUS inspired multi-upload support #4188
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
Open
LukasMalyszko
wants to merge
73
commits into
master
Choose a base branch
from
upload-support
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
73 commits
Select commit
Hold shift + click to select a range
4060c70
Add TUS upload support and related configurations
LukasMalyszko 6d7bb11
Refactor TUS upload handling and enhance test coverage
LukasMalyszko 7313fa7
Implement validation for TUS upload IDs and enhance error handling
LukasMalyszko 6ab0e2c
Enhance TUS upload tests and ensure proper file handling
LukasMalyszko 89c89dc
Refactor TUS upload handling and improve multi-upload file input mana…
LukasMalyszko d86bf9d
Refactor upload URL generation and improve test clarity
LukasMalyszko 5f7e174
Refactor TUS upload handling and simplify related settings
LukasMalyszko e3231ee
Enhance multi-upload controller and file upload form for TUS support
LukasMalyszko 33068d8
Enhance multi-upload functionality with improved TUS support
LukasMalyszko 5a0f8f3
Update footer display in file upload tests to reflect max size per fi…
LukasMalyszko 9230f7b
Enhance TUS upload handling and improve file upload form
LukasMalyszko 1b04e9d
Merge branch 'master' into upload-support
LukasMalyszko 3c1b47b
Add tests for TUS upload error handling and configuration validation
LukasMalyszko 5d8f97e
Enhance TUS upload functionality and configuration
LukasMalyszko d289569
Refactor TUS upload handling and improve error responses
LukasMalyszko 29584b1
Refactor Upload-Offset validation in TUS uploads
LukasMalyszko 6be83ea
Merge branch 'master' into upload-support
LukasMalyszko c9bf1fa
Add tests for TUS upload filename handling and response behavior
LukasMalyszko 2de297d
Merge branch 'master' into upload-support
LukasMalyszko 7b534d0
Refactor file count management in multi-upload controller
LukasMalyszko 0544692
Increase wait times for TUS upload completion in tests
LukasMalyszko 4586e39
Add trigger for file input change event in upload tests
LukasMalyszko 1680da0
Refactor file count updates in multi-upload controller
LukasMalyszko 98afbf9
Enhance TUS upload handling for chunked transfers
LukasMalyszko 2550b85
Implement upload progress handling and block push creation during upl…
LukasMalyszko 7c60f16
Remove disabled state check for submit button in file upload tests
LukasMalyszko 5c7b579
Update file upload test to assert button disabled state during upload…
LukasMalyszko cc4ad8f
Update file upload test to assert button enabled state after upload c…
LukasMalyszko 1e70c54
Enhance TUS upload security and improve file upload handling
LukasMalyszko 424ccdd
Update file upload test to use a larger file for better observation o…
LukasMalyszko 534bbb8
Enhance TUS upload handling and file type validation
LukasMalyszko c40a505
Add content type sanitation method to TusUploadsController
LukasMalyszko 3cf5be3
Reset TUS upload count on new and edit push forms
LukasMalyszko 1b8deee
Refactor file upload test to remove unnecessary disabled state check
LukasMalyszko b8ebe5b
Merge remote-tracking branch 'origin/master' into upload-support
LukasMalyszko 00a659c
Enhance file upload form with TUS and direct upload support
LukasMalyszko 606fd87
Refactor user confirmation in tests
LukasMalyszko 746c751
Update visibility option in file push upload UI test
LukasMalyszko fc3d1b1
Update assertion in file push upload UI test to reflect new success m…
LukasMalyszko 8259ea5
Merge remote-tracking branch 'origin/master' into upload-support
LukasMalyszko bcdb040
Fix tus_uploads_enabled? method to check for disabled logins
LukasMalyszko a9af142
Update test settings to use disable_logins instead of enable_logins
LukasMalyszko dfd83c6
Refactor TUS upload handling and improve UI for file uploads
LukasMalyszko 9ff68c0
Refactor multi-upload controller to support asynchronous TUS uploads
LukasMalyszko 76379b2
Enhance file size parsing and improve TUS upload handling
LukasMalyszko 0985446
- Removed the constant for maximum active TUS session IDs and replace…
LukasMalyszko 552b196
Refactor TUS upload size handling and improve configuration
LukasMalyszko bf01101
Enhance direct upload size handling and improve configuration
LukasMalyszko 2369f6e
Merge remote-tracking branch 'origin/master' into upload-support
LukasMalyszko e2e02f3
Refactor TUS upload handling and improve error logging
LukasMalyszko 14cc44f
Fix missing newline at end of files in controllers and services
LukasMalyszko 6706d99
Refactor push update logic to improve error handling
LukasMalyszko 4109aff
Merge remote-tracking branch 'origin/master' into upload-support
LukasMalyszko 93cce3f
Update yarn.lock to include new dependencies and versions
LukasMalyszko 5316cd7
Refactor TUS upload integration and clean up asset management
LukasMalyszko 90954a3
Update import map for TUS client to include detailed comment on loadi…
LukasMalyszko 5e11bd6
Refactor TUS upload controller and multi-upload logic
LukasMalyszko 3ab2429
Remove TUS-related tests and settings from ApplicationHelperTest
LukasMalyszko e8fa835
Remove FORCE_SSL environment variable in ApplicationHelperTest setup
LukasMalyszko 5114674
Enhance TUS upload handling with user ownership checks and session li…
LukasMalyszko 6bc1118
Merge remote-tracking branch 'origin/master' into upload-support
LukasMalyszko dcfc423
Refactor user ID retrieval and enhance test teardown logic
LukasMalyszko 828ea86
Refactor error handling in TUS upload destruction logic
LukasMalyszko ca41f3f
increase readability of code
LukasMalyszko 6e0ed5f
Enhance multi-upload controller with new error message handling
LukasMalyszko f228c1b
Enhance accessibility in multi-upload controller
LukasMalyszko eeb7464
Remove unused tusServerUploadId references in multi-upload controller
LukasMalyszko db681fe
Merge remote-tracking branch 'origin/master' into upload-support
LukasMalyszko 2bfde7f
Refactor push creation and update logic to simplify upload checks
LukasMalyszko 7df866f
Refactor upload checks in TusUploadsController to streamline not foun…
LukasMalyszko 7c6eba8
Remove unused size parsing logic from ApplicationHelper to improve co…
LukasMalyszko 62261a9
fix
LukasMalyszko fb1fc75
Merge branch 'master' into upload-support
LukasMalyszko File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| # Tracks in-flight TUS uploads per session so file push create/update can wait for completion. | ||
| # Uses upload IDs (not a simple counter) so abandoned or failed uploads can be released via | ||
| # DELETE /uploads/:id without leaving a stale positive count. | ||
| module TusActiveUploadSession | ||
| extend ActiveSupport::Concern | ||
|
|
||
| MAX_TRACKED_SESSION_UPLOADS = 5 | ||
|
|
||
| private | ||
|
|
||
| def tus_upload_id_list | ||
| session[:tus_active_upload_ids] ||= [] | ||
| session.delete(:tus_upload_count) if session.key?(:tus_upload_count) | ||
| session[:tus_active_upload_ids] | ||
| end | ||
|
|
||
| def register_tus_upload_in_session!(id) | ||
| list = tus_upload_id_list | ||
| return if list.include?(id) | ||
| # Defense in depth: create also rejects when the session is full (409 before register). | ||
| return if list.size >= MAX_TRACKED_SESSION_UPLOADS | ||
|
|
||
| list << id | ||
| session[:tus_active_upload_ids] = list | ||
| end | ||
|
|
||
| def release_tus_upload_from_session!(id) | ||
| list = tus_upload_id_list | ||
| list.delete(id) | ||
| session[:tus_active_upload_ids] = list | ||
| end | ||
|
|
||
| def tus_uploads_in_progress? | ||
| tus_upload_id_list.any? | ||
| end | ||
|
|
||
| def reset_tus_upload_session! | ||
| session.delete(:tus_upload_count) | ||
| session[:tus_active_upload_ids] = [] | ||
| end | ||
|
|
||
| def tus_upload_session_tracked?(id) | ||
| tus_upload_id_list.include?(id) | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,273 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| # Minimal TUS-inspired resumable upload endpoint. | ||
| # POST create, PATCH append, HEAD status. Finalize creates ActiveStorage::Blob and returns signed_id. | ||
| # CSRF is skipped for the TUS flow; we mitigate session-poisoning (cross-site POST registering | ||
| # bogus upload IDs) by requiring Origin/Referer to match the request host when present. | ||
| class TusUploadsController < ApplicationController | ||
| include TusActiveUploadSession | ||
|
|
||
| skip_before_action :verify_authenticity_token, only: %i[create update destroy] | ||
| before_action :require_tus_enabled | ||
| before_action :authenticate_user! | ||
| before_action :reject_cross_origin_tus_requests | ||
|
|
||
| # POST /uploads | ||
| def create | ||
| upload_length = request.headers["Upload-Length"]&.to_i | ||
| if upload_length.blank? || upload_length <= 0 | ||
| return head :bad_request | ||
| end | ||
|
|
||
| max_size = helpers.max_tus_upload_size_bytes | ||
| if max_size.positive? && upload_length > max_size | ||
| return head :payload_too_large | ||
| end | ||
|
|
||
| filename, content_type = parse_upload_metadata(request.headers["Upload-Metadata"]) | ||
|
|
||
| if tus_upload_id_list.size >= TusActiveUploadSession::MAX_TRACKED_SESSION_UPLOADS | ||
| return head :conflict | ||
| end | ||
|
|
||
| id = TusUploadStore.generate_id | ||
| store = TusUploadStore.new(id) | ||
| store.create!( | ||
| upload_length: upload_length, | ||
| user_id: current_user.id, | ||
| filename: filename, | ||
| content_type: content_type | ||
| ) | ||
|
|
||
| register_tus_upload_in_session!(id) | ||
|
|
||
| # Use relative URL so the client (browser) sends PATCH to the same origin | ||
|
LukasMalyszko marked this conversation as resolved.
|
||
| response.headers["Location"] = upload_path(id) | ||
| response.headers["Upload-Offset"] = "0" | ||
| response.headers["Upload-Length"] = upload_length.to_s | ||
| head :created | ||
| end | ||
|
|
||
| # PATCH /uploads/:id — append chunk | ||
| # HEAD /uploads/:id — status | ||
| def update | ||
| id = params[:id] | ||
| return head :not_found unless TusUploadStore.valid_id?(id) | ||
|
|
||
| store = TusUploadStore.new(id) | ||
| return handle_tus_head(store) if request.head? | ||
| return head :method_not_allowed unless request.patch? | ||
|
|
||
| max_chunk = helpers.tus_chunk_size_bytes | ||
| return head :payload_too_large if tus_patch_content_length_exceeds_chunk_limit?(max_chunk) | ||
| # When Content-Length is absent (e.g. chunked encoding), we still enforce max_chunk | ||
| # in the store by limiting how many bytes we read from the body. | ||
|
|
||
| return respond_tus_patch_nonexistent_store(id) unless store.exist? | ||
|
|
||
| return head :not_found unless tus_upload_owned_by_current_user?(store) | ||
| return head :gone if store.complete? | ||
|
|
||
| expected_offset = parse_upload_offset_header | ||
| return head :bad_request if expected_offset.nil? | ||
|
|
||
| new_offset = append_tus_chunk_or_abort!(store, expected_offset, max_chunk) | ||
| return if performed? | ||
|
|
||
| if store.complete? | ||
| finalize_and_cache_tus_upload!(store, id, new_offset) | ||
| return | ||
| end | ||
|
|
||
| assign_tus_progress_headers!(offset: new_offset) | ||
| head :no_content | ||
| end | ||
|
|
||
| # DELETE /uploads/:id — abandon in-progress upload; drops temp files and clears session tracking. | ||
| def destroy | ||
| id = params[:id].to_s | ||
| unless TusUploadStore.valid_id?(id) | ||
| return head :not_found | ||
| end | ||
|
|
||
| store = TusUploadStore.new(id) | ||
| return head :not_found unless store.exist? | ||
| return head :not_found unless tus_upload_owned_by_current_user?(store) | ||
|
|
||
| store.destroy! | ||
| release_tus_upload_from_session!(id) | ||
| head :no_content | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def handle_tus_head(store) | ||
| return head :not_found unless store.exist? | ||
| return head :not_found unless tus_upload_owned_by_current_user?(store) | ||
|
|
||
| assign_tus_progress_headers!(offset: store.upload_offset, length: store.upload_length) | ||
| head :no_content | ||
| end | ||
|
|
||
| def tus_patch_content_length_exceeds_chunk_limit?(max_chunk) | ||
| content_length = request.content_length | ||
| return false unless content_length.present? && max_chunk.positive? | ||
|
|
||
| content_length > max_chunk | ||
| end | ||
|
|
||
| def respond_tus_patch_nonexistent_store(id) | ||
| # Retry of final PATCH after we already finalized: return success so client gets X-Signed-Id | ||
| finalized = finalized_upload_cache_read(id) | ||
| if finalized && finalized_upload_cache_owned_by_current_user?(finalized) | ||
| assign_tus_progress_headers!( | ||
| offset: finalized["upload_offset"], | ||
| length: finalized["upload_length"], | ||
| signed_id: finalized["signed_id"] | ||
| ) | ||
| head :no_content | ||
| else | ||
| head :not_found | ||
| end | ||
| end | ||
|
|
||
| def parse_upload_offset_header | ||
| raw = request.headers["Upload-Offset"] | ||
| return nil if raw.blank? || !raw.to_s.strip.match?(/\A\d+\z/) | ||
|
|
||
| raw.to_s.strip.to_i | ||
| end | ||
|
|
||
| def append_tus_chunk_or_abort!(store, expected_offset, max_chunk) | ||
| store.append_chunk!( | ||
| offset: expected_offset, | ||
| io: request.body, | ||
| max_bytes: max_chunk.positive? ? max_chunk : nil | ||
| ) | ||
| rescue TusUploadStore::OffsetMismatch => e | ||
| response.headers["Upload-Offset"] = e.current_offset.to_s | ||
| head :conflict | ||
| nil | ||
| rescue TusUploadStore::NotFound | ||
| head :not_found | ||
| nil | ||
| end | ||
|
|
||
| def finalize_and_cache_tus_upload!(store, id, new_offset) | ||
| upload_length = store.upload_length | ||
| blob = store.finalize_to_blob! | ||
| release_tus_upload_from_session!(id) | ||
| finalized_upload_cache_write(id, signed_id: blob.signed_id, upload_length: upload_length, upload_offset: new_offset, user_id: current_user.id) | ||
| assign_tus_progress_headers!(offset: new_offset, length: upload_length, signed_id: blob.signed_id) | ||
| head :no_content | ||
| rescue TusUploadStore::NotFound | ||
| head :not_found | ||
| rescue ArgumentError => e | ||
| return head :gone if e.message&.include?("upload not complete") | ||
|
|
||
| raise | ||
| end | ||
|
|
||
| # Sets Upload-Offset always; Upload-Length and X-Signed-Id only when provided (partial PATCH omits length). | ||
| def assign_tus_progress_headers!(offset:, length: nil, signed_id: nil) | ||
| response.headers["Upload-Offset"] = offset.to_s | ||
| response.headers["Upload-Length"] = length.to_s unless length.nil? | ||
| response.headers["X-Signed-Id"] = signed_id if signed_id | ||
| end | ||
|
|
||
| def reject_cross_origin_tus_requests | ||
| origin = request.headers["Origin"].presence | ||
| referer = request.headers["Referer"].presence | ||
| return if origin.blank? && referer.blank? # Allow when neither sent (e.g. same-origin no Origin) | ||
|
|
||
| allowed_host = request.host | ||
|
|
||
| if origin.present? | ||
| return head :forbidden unless header_origin_host_allowed?(origin, allowed_host) | ||
| end | ||
| if referer.present? | ||
| head :forbidden unless header_origin_host_allowed?(referer, allowed_host) | ||
| end | ||
| end | ||
|
|
||
| def header_origin_host_allowed?(header_value, allowed_host) | ||
| uri = URI.parse(header_value) | ||
| uri.host&.downcase == allowed_host.downcase | ||
| rescue URI::InvalidURIError | ||
| false | ||
| end | ||
|
|
||
| def require_tus_enabled | ||
| return if helpers.tus_uploads_enabled? | ||
| head :not_found | ||
| end | ||
|
|
||
| def finalized_upload_cache_read(upload_id) | ||
| Rails.cache.read("tus_finalized/#{upload_id}") | ||
| end | ||
|
|
||
| def finalized_upload_cache_write(upload_id, signed_id:, upload_length:, upload_offset:, user_id:) | ||
| Rails.cache.write( | ||
| "tus_finalized/#{upload_id}", | ||
| { | ||
| "signed_id" => signed_id, | ||
| "upload_length" => upload_length, | ||
| "upload_offset" => upload_offset, | ||
| "user_id" => user_id | ||
| }, | ||
| expires_in: 2.minutes | ||
| ) | ||
| end | ||
|
|
||
| def tus_upload_owned_by_current_user?(store) | ||
| owner_id = store.meta_upload_user_id | ||
| owner_id.present? && owner_id == current_user.id | ||
| end | ||
|
|
||
| # Used when tmp data is gone but the client retries the final PATCH; cache must match + current user. | ||
| def finalized_upload_cache_owned_by_current_user?(finalized) | ||
| finalized["user_id"].to_i == current_user.id | ||
| end | ||
|
|
||
| # TUS Upload-Metadata: comma-separated "key base64value" pairs | ||
| def parse_upload_metadata(header) | ||
| filename = nil | ||
| content_type = nil | ||
| return [filename, content_type] if header.blank? | ||
| header.split(",").each do |part| | ||
| key, value = part.strip.split(/\s+/, 2) | ||
| next if value.blank? | ||
| decoded = begin | ||
| Base64.strict_decode64(value) | ||
| rescue ArgumentError | ||
| nil | ||
| end | ||
| next unless decoded | ||
| case key&.downcase | ||
| when "filename" then filename = sanitize_upload_filename(decoded) | ||
| when "filetype" then content_type = sanitize_upload_content_type(decoded) | ||
| end | ||
| end | ||
| [filename.presence, content_type.presence] | ||
| end | ||
|
|
||
| TUS_CONTENT_TYPE_BLOCKLIST = %w[text/html text/javascript application/javascript application/x-javascript application/ecmascript text/vbscript].freeze | ||
|
|
||
| def sanitize_upload_content_type(value) | ||
| return nil if value.blank? | ||
| type = value.to_s.strip.downcase.split(/\s*;\s*/).first | ||
| return nil if type.blank? | ||
| return nil if TUS_CONTENT_TYPE_BLOCKLIST.include?(type) | ||
| type.presence | ||
| end | ||
|
|
||
| def sanitize_upload_filename(name) | ||
| return if name.blank? | ||
| sanitized = name.strip | ||
| # Remove any path components (handles both Unix and Windows-style paths) | ||
| sanitized = File.basename(sanitized) | ||
| # Replace any characters that are not alphanumeric, dot, dash, plus, or underscore | ||
| sanitized = sanitized.gsub(/[^a-zA-Z0-9.\-+_]/, "_") | ||
| sanitized.presence | ||
| end | ||
| end | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
When I was uploading a file, I refreshed the page. So,
session[:tus_upload_count]stayed as 1. Then, I tried to create a file push, and it showed this error. Becausesession[:tus_upload_count]is 1. I solved this issue by removing session data.If
:tus_upload_countis removed fromsession, may it cause any issue?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.
Yes session should be removed when refresh, Thanks for catching this. It need a fix for sure.
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