OF-31: Preserve conference service context in room creation flow#3376
OF-31: Preserve conference service context in room creation flow#3376MilanTyagi2004 wants to merge 3 commits into
Conversation
When creating a new MUC room from a service-specific administration page, the room creation form incorrectly defaulted to the alphabetically first conference service. Extract the active service context from the originating request and pass it through the room creation flow so that the selected conference service is pre-populated when available. Falls back to the existing default behavior when no valid service context can be determined.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR modifies the MUC room creation workflow to enable explicit service context propagation. The room-create page now reads the HTTP Referer header to extract service context (mucname, mucName, or roomJID domain), encodes it as a query parameter, and redirects to the room-edit-form page. The edit form page accepts this context parameter, validates it, and uses it to resolve the appropriate MUC service for room configuration instead of always selecting the first registered service. Copyright years are updated in both files. Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
xmppserver/src/main/webapp/muc-room-create.jsp (1)
33-48: ⚡ Quick winConsider URL decoding and encoding the extracted parameter values for consistency and robustness.
The
mucnameandmucNameparameter values are extracted from the Referer header and appended directly to the redirect URL without explicit decoding/encoding (lines 39, 47). While this pass-through approach may work in typical cases, it's inconsistent with theroomJIDhandling (lines 57-60) which explicitly decodes and encodes the value.Explicit decode/encode would make the code more robust against edge cases with special characters and maintain consistency across all extraction branches.
♻️ Proposed refactor for explicit decode/encode
String val = endIdx != -1 ? referrer.substring(idx + 8, endIdx) : referrer.substring(idx + 8); - serviceParam = "&mucName=" + val; + try { + val = java.net.URLDecoder.decode(val, java.nio.charset.StandardCharsets.UTF_8.name()); + serviceParam = "&mucName=" + java.net.URLEncoder.encode(val, java.nio.charset.StandardCharsets.UTF_8.name()); + } catch (Exception e) { + // Ignore parsing errors + }Apply the same pattern to both the
mucname(line 39) andmucName(line 47) branches.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@xmppserver/src/main/webapp/muc-room-create.jsp` around lines 33 - 48, The mucname/mucName extraction branches currently append raw substrings from the referrer into serviceParam; change them to follow the same decode/encode pattern used for roomJID: after extracting val from referrer (from referrer.substring(...)), first URL-decode it (using URLDecoder.decode with UTF-8), then URL-encode the decoded value (using URLEncoder.encode with UTF-8) before assigning serviceParam = "&mucName=" + encodedVal; update both the "mucname" and "mucName" branches and reuse the same referrer/val/serviceParam variables to keep behavior consistent with the roomJID handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@xmppserver/src/main/webapp/muc-room-create.jsp`:
- Around line 33-48: The mucname/mucName extraction branches currently append
raw substrings from the referrer into serviceParam; change them to follow the
same decode/encode pattern used for roomJID: after extracting val from referrer
(from referrer.substring(...)), first URL-decode it (using URLDecoder.decode
with UTF-8), then URL-encode the decoded value (using URLEncoder.encode with
UTF-8) before assigning serviceParam = "&mucName=" + encodedVal; update both the
"mucname" and "mucName" branches and reuse the same referrer/val/serviceParam
variables to keep behavior consistent with the roomJID handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 64d4253e-f775-4e46-98f8-8899676cc455
📒 Files selected for processing (2)
xmppserver/src/main/webapp/muc-room-create.jspxmppserver/src/main/webapp/muc-room-edit-form.jsp
Apply URL decode/encode normalization to mucname and mucName parameters extracted from the Referer header. This aligns their handling with the existing roomJID path and ensures consistent processing of URL-encoded values before forwarding them to the room creation form.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
xmppserver/src/main/webapp/muc-room-create.jsp (1)
33-80: ⚡ Quick winDuplicate
mucname/mucNamedecode-encode branches should be unifiedThe two branches are identical apart from key name. Consolidating them into one extraction/normalization path will reduce maintenance drift and keep behavior consistent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@xmppserver/src/main/webapp/muc-room-create.jsp` around lines 33 - 80, Unify the duplicate branches that extract and normalize the query parameter by detecting either key; search for the logic using referrer and serviceParam where it handles "mucname=" and "mucName=" and replace both conditional blocks with a single extraction/normalization path: locate the index by checking for "mucname=" or "mucName=" (e.g., set a variable key = foundKey), compute idx/endIdx and val once, then perform the URLDecoder/URLEncoder steps on val to produce encodedVal and assign serviceParam = "&mucName=" + encodedVal; keep the existing try/catch and validation fallback behavior and reuse decodedVal/encodedVal variable names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@xmppserver/src/main/webapp/muc-room-create.jsp`:
- Around line 33-43: The current referrer parsing (uses
referrer.contains("mucname=") + indexOf/substr) can match inside other keys
(e.g., "notmucname"); change to extract query params by key boundaries: look for
either start-of-query (?) or '&' before the key and '=' after it, or better yet
implement/reuse a small helper to parse the query string and return the value
for a given key (operate on the referrer variable and replace the direct
contains/indexOf logic for "mucname=" and the other two occurrences); ensure you
stop at '&' or '#' or end-of-string and only accept matches where the key is
isolated (preceded by '?' or '&') so incorrect substrings are not captured.
---
Nitpick comments:
In `@xmppserver/src/main/webapp/muc-room-create.jsp`:
- Around line 33-80: Unify the duplicate branches that extract and normalize the
query parameter by detecting either key; search for the logic using referrer and
serviceParam where it handles "mucname=" and "mucName=" and replace both
conditional blocks with a single extraction/normalization path: locate the index
by checking for "mucname=" or "mucName=" (e.g., set a variable key = foundKey),
compute idx/endIdx and val once, then perform the URLDecoder/URLEncoder steps on
val to produce encodedVal and assign serviceParam = "&mucName=" + encodedVal;
keep the existing try/catch and validation fallback behavior and reuse
decodedVal/encodedVal variable names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5398158e-c9fa-4c6e-9a6b-7bc5f179aa82
📒 Files selected for processing (1)
xmppserver/src/main/webapp/muc-room-create.jsp
| ); | ||
| String encodedVal = java.net.URLEncoder.encode( | ||
| decodedVal, | ||
| java.nio.charset.StandardCharsets.UTF_8.name() |
There was a problem hiding this comment.
nitpick: shorter URLEncoder.encode(decodedVal, StandardCharsets.UTF_8) feel free to static import the constant URLEncoder.encode(decodedVal, UTF_8)
- Normalize mucname and mucName values using URL decode/encode to ensure consistent handling of URL-encoded parameters. - Replace substring-based Referer parsing with key-bound query parameter extraction to prevent false matches from similarly named parameters. - Use regex-based parameter extraction for mucname, mucName, and roomJID values. - Simplify URL encoding/decoding usage through imports. - Preserve existing fallback behavior when service context cannot be resolved.
Summary
Fixes OF-31 by preserving the current conference service context when creating a new MUC room.
Previously, when an administrator navigated to Create New Room from a service-specific MUC administration page, the room creation form defaulted to the alphabetically first conference service because the selected service context was lost during navigation.
This change derives the active conference service from the originating request and forwards it through the room creation flow, allowing the room creation form to preselect the correct service and load the appropriate service-specific defaults.
Root Cause
The Create New Room sidebar entry is a top-level navigation item generated from a static URL. As a result, service-specific request parameters (
mucname,mucName, orroomJID) were not carried to the room creation flow.When
muc-room-edit-form.jspwas opened without service context, it defaulted to the first available conference service.Changes
Updated
muc-room-create.jspto derive the active conference service from the originating request and forward it to the room creation form.Updated
muc-room-edit-form.jspto:mucNameandmucnameparameters.Backward Compatibility
No behavior changes occur when service context is unavailable.
The room creation form continues to default to the first available conference service in the following cases:
Testing
Verified with multiple conference services configured:
Open a service-specific MUC administration page (e.g.
conference2).Click Create New Room.
Confirm that:
conference2.Verify that room creation succeeds under the selected service.
Verify that navigation from non-service-specific pages continues to use the existing default service selection behavior.