[MBL-19470][S/T/P] Fixes issue of un-viewable file upload submissions for assignments with moderated grading #3760

suhaibabsi-inst · 2025-11-18T08:54:12Z

refs: MBL-19470
affects: All
builds: All
release note: Fixed issue of un-viewable file upload submissions for assignments with moderated grading

Analysis

The issue was caused by double percent-encoding occurring while decoding APIFile.preview_url:
The parsed string is usually get percent-encoded on backend, but for the failing case the string was containing query-unsafe characters [ & ], which will be percent- encoded when fed to URL.init(string:), but the unwanted result was to double encode the already encoded characters:

For simplicity sake, a path like this:
/api/v1/canvadoc_session?blob=[Some%20Example]

Would turn to be:
/api/v1/canvadoc_session?blob=%5BSome%2520Example%5D

Upon that request always fail with 401 Unauthorized error.

The Fix

While that could be fixed from backend, I added few changes to remove the percent-encoding on the query part of the string, so it can be encoded again as a whole when using URL(string:relativeTo:) with the resulting one. Assuming BE only perform 1 iteration of percent-encoding.

Test Plan

See ticket's description for test account.
Also made sure the original case of file upload submissions still work as expected.

Checklist

refs: MBL-19470 affects: All builds: All release note: Fixed issue of un-viewable file upload submissions for assignments with moderated grading

claude · 2025-11-18T08:55:09Z

Claude Code Review

Updated: $(date)

Critical Issues Found:

Flawed hasPercentEncoding logic (line 76-79): When removingPercentEncoding returns nil, the decoded variable becomes self, making the comparison always return false. This breaks detection for strings without percent encoding. The check will incorrectly skip encoding on non-encoded brackets.
Uncontrolled bracket replacement (line 89-93): Replaces literal [ and ] characters unconditionally when any percent-encoding is detected. This is too broad - brackets in non-query parts of URLs or already-encoded brackets could be affected incorrectly.
Incomplete handling of existing encoded brackets (line 91): Code doesn't check if brackets are already encoded (e.g., %5B, %5D) before replacing. A string with %5Bvalue%5D would incorrectly attempt replacement.

🔴 Issues found

inst-danger · 2025-11-18T09:05:13Z

	Warnings
⚠️	One or more files are below the minimum test coverage 50%
⚠️	The total test coverage is below the minimum 90%

Release Note:

Fixed issue of un-viewable file upload submissions for assignments with moderated grading

Affected Apps: Student, Teacher, Parent

Builds: Student, Teacher, Parent

MBL-19470

Coverage	New %	Master %	Delta
Canvas iOS	81.07%	81.06%	0.01%
Horizon/Horizon/Sources/Features/Dashboard/SkillsHighlightsWidget/Data/ProficiencyLevel.swift	0%	0%	0%
Horizon/Horizon/Sources/Features/Notebook/Common/View/HighlightWebView/EnableZoom.swift	0%	0%	0%
Horizon/Horizon/Sources/Features/Notebook/Common/View/HighlightWebView/HighlightWebFeature.swift	0%	0%	0%
Horizon/Horizon/Sources/Common/Domain/GetCoursesInteractor.swift	38.16%	38.16%	0%
Horizon/Horizon/Sources/Features/LearningObjects/Assignment/SubmissionComment/Data/CommentAttachment.swift	0%	0%	0%
Horizon/Horizon/Sources/Features/Account/Notifications/Domain/NotificationSettingsInteractor.swift	0%	0%	0%
Horizon/Horizon/Sources/Common/Data/HModuleStatus.swift	0%	0%	0%
Horizon/Horizon/Sources/Features/Session/SessionInteractor.swift	33.33%	33.33%	0%
Horizon/Horizon/Sources/Common/Utilities/Double+.swift	0%	0%	0%
Horizon/Horizon/Sources/Features/Assist/AssistFlashCard/View/AssistFlashCardViewModel.swift	0%	0%	0%

Generated by 🚫 dangerJS against adbc294

inst-danger · 2025-11-18T09:26:01Z

Builds

Commit: SwiftLint fixes (adbc294)
Build Number: 784
Built At: Nov 18 12:17 CET (11/18 04:17 AM MST)

Student	Teacher	Parent

refs: MBL-19470 affects: Student, Teacher, Parent builds: Student, Teacher, Parent release note: Fixed issue of un-viewable file upload submissions for assignments with moderated grading

claude · 2025-11-18T10:06:27Z

Claude Code Review

Analysis

Changes: Fixed double percent-encoding in URL query strings by decoding query parameters before URL construction.

Issues Found:

Potential Bug - Deep Percent Decoding Loop: The removingPercentEncodingDeeply method (lines 84-90) repeatedly calls removingPercentEncoding until the value stops changing. This could cause unexpected data loss if legitimate percent-encoded characters exist multiple layers deep. The fix assumes only backend encoding layers exist, which may not hold for all edge cases.
Missing Fragment Handling: The fix only addresses query components. If URL fragments contain double-encoded content, they won't be corrected.
Silent Fallback: If URLComponents initialization fails, the method silently returns the original unchanged string. This masks malformed URLs and makes them appear valid.
Limited Test Coverage: Tests lack edge cases such as consecutive percent-encoded characters (e.g., %25%20), URLs without queries, or malformed encoding.

✅ Code compiles, follows project patterns, and addresses the primary use case for moderated grading file uploads.

claude · 2025-11-18T10:23:48Z

Claude Code Review

Updated: 2025-11-18

✅ Approval

The fix correctly addresses the double percent-encoding issue by stripping percent-encoding from query parameters before URL construction.

Key points:

Approach is sound: decode query parameters fully, then let URL init handle fresh encoding
Recursive removingPercentEncodingDeeply is safe (properly guarded against nil)
Test coverage includes the problematic bracket cases and multiple edge cases
Fallback chain is robust (try direct URL, then percent-encode if needed)
Scope is appropriate (helpers are fileprivate/private)

No critical issues found. ✅

claude · 2025-11-18T10:32:23Z

Claude Code Review

Updated: 2025-11-18

✅ Approved

Query parameter decoding before URL construction prevents double encoding
Fallback approach is sound: decode once, let URL init handle fresh encoding
Test coverage covers bracket cases and mixed encoding scenarios
Implementation is properly scoped (fileprivate helpers)

No critical issues found.

suhaibabsi-inst · 2025-11-18T10:38:01Z

Core/Core/Common/CommonModels/API/APIURL.swift

+    /// percent-encoding on URL strings.
+    var removingQueryPercentEncoding: String {
+        if var comps = URLComponents(string: self) {
+            if let fixedQuery = comps.query?.removingPercentEncoding {


It is better to assume 1 level of percent-encoding on backend.
Since, we don't want for the excessive percent-encoding removal to turn such value as:

"{ \"prop\": \"%5BSome%20Value%5D\" }"

To this:

"{ \"prop\": \"[Some Value]\" }"

claude · 2025-11-18T10:44:32Z

Claude Code Review

Updated: 2025-11-18

✅ Approved

No critical issues found. The fix correctly addresses the double percent-encoding problem with solid test coverage.

Fix URL double percent encoding

35d611e

refs: MBL-19470 affects: All builds: All release note: Fixed issue of un-viewable file upload submissions for assignments with moderated grading

suhaibabsi-inst self-assigned this Nov 18, 2025

suhaibabsi-inst changed the title ~~Fix URL double percent encoding~~ [MBL-19470][S/T/P] Fixes issue of un-viewable file upload submissions for assignments with moderated grading Nov 18, 2025

Unit tests

23371ae

Fixing query percent encoding

84597e7

refs: MBL-19470 affects: Student, Teacher, Parent builds: Student, Teacher, Parent release note: Fixed issue of un-viewable file upload submissions for assignments with moderated grading

More unit tests

a1819b5

Address AI comments

6f240f1

suhaibabsi-inst commented Nov 18, 2025

View reviewed changes

SwiftLint fixes

adbc294

suhaibabsi-inst marked this pull request as ready for review November 18, 2025 11:04

suhaibabsi-inst requested review from rh12 and vargaat as code owners November 18, 2025 11:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[MBL-19470][S/T/P] Fixes issue of un-viewable file upload submissions for assignments with moderated grading #3760

[MBL-19470][S/T/P] Fixes issue of un-viewable file upload submissions for assignments with moderated grading #3760

Uh oh!

suhaibabsi-inst commented Nov 18, 2025 •

edited

Loading

Uh oh!

claude bot commented Nov 18, 2025 •

edited

Loading

Uh oh!

inst-danger commented Nov 18, 2025 •

edited by atlassian bot

Loading

Uh oh!

inst-danger commented Nov 18, 2025 •

edited

Loading

Uh oh!

claude bot commented Nov 18, 2025

Uh oh!

claude bot commented Nov 18, 2025

Uh oh!

claude bot commented Nov 18, 2025

Uh oh!

suhaibabsi-inst Nov 18, 2025

Uh oh!

claude bot commented Nov 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[MBL-19470][S/T/P] Fixes issue of un-viewable file upload submissions for assignments with moderated grading #3760

Are you sure you want to change the base?

[MBL-19470][S/T/P] Fixes issue of un-viewable file upload submissions for assignments with moderated grading #3760

Uh oh!

Conversation

suhaibabsi-inst commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Analysis

The Fix

Test Plan

Checklist

Uh oh!

claude bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review

Critical Issues Found:

Uh oh!

inst-danger commented Nov 18, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release Note:

Affected Apps: Student, Teacher, Parent

Builds: Student, Teacher, Parent

Uh oh!

inst-danger commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Builds

Uh oh!

claude bot commented Nov 18, 2025

Claude Code Review

Analysis

Uh oh!

claude bot commented Nov 18, 2025

Claude Code Review

✅ Approval

Uh oh!

claude bot commented Nov 18, 2025

Claude Code Review

Uh oh!

suhaibabsi-inst Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

claude bot commented Nov 18, 2025

Claude Code Review

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

suhaibabsi-inst commented Nov 18, 2025 •

edited

Loading

claude bot commented Nov 18, 2025 •

edited

Loading

inst-danger commented Nov 18, 2025 •

edited by atlassian bot

Loading

inst-danger commented Nov 18, 2025 •

edited

Loading