Feature/signaturerelease#103
Conversation
WalkthroughUpdates adjust file download response headers by URL-encoding filenames and constructing ResponseEntity with explicit headers, media type, and content length. Additionally, the user model changes a supervisor getter to return a Boolean and makes the deleted getter null-safe. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant Cn as EmployeeSignatureController
participant S as Repository/Service
participant R as ResponseEntity
C->>Cn: GET /employee-signature/{userId}
Cn->>S: Fetch user signature + metadata
S-->>Cn: byte[] signature, mediaType, filename
note right of Cn: URL-encode filename<br/>Build headers: Content-Disposition<br/>Set media type and content length
Cn->>R: Create ResponseEntity<byte[]>
R-->>C: 200 OK with attachment
alt Error (e.g., invalid media type)
Cn-->>C: Error ResponseEntity
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/iemr/admin/data/user/M_User.java (2)
93-93: Bug: field shadowing leavesuserIDuninitialized
userID = userID;assigns the parameter to itself. The field remains defaulted.- userID = userID; + this.userID = userID;
62-65: Prevent password exposure in toString()- private String Password; + @com.fasterxml.jackson.annotation.JsonIgnore + @Expose(serialize = false, deserialize = false) + private String Password;OutputMapper.gson() is already configured with
excludeFieldsWithoutExposeAnnotation(), sotoString()will omit non-exposed fields.
🧹 Nitpick comments (1)
src/main/java/com/iemr/admin/controller/employeemaster/EmployeeSignatureController.java (1)
108-115: Response construction LGTM; expose header for CORS and disable caching (optional)Add exposure so browsers can read
Content-Dispositionacross origins and prevent caching of personal artifacts.byte[] fileBytes = userSignID.getSignature(); // MUST be byte[] - return ResponseEntity.ok() + // Let browsers access the filename on cross-origin downloads + responseHeaders.add(HttpHeaders.ACCESS_CONTROL_EXPOSE_HEADERS, HttpHeaders.CONTENT_DISPOSITION); + // Avoid caching user signatures + responseHeaders.setCacheControl("no-store"); + + return ResponseEntity.ok() .headers(responseHeaders) .contentType(mediaType) .contentLength(fileBytes.length) .body(fileBytes);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/com/iemr/admin/controller/employeemaster/EmployeeSignatureController.java(2 hunks)src/main/java/com/iemr/admin/data/user/M_User.java(2 hunks)
🔇 Additional comments (3)
src/main/java/com/iemr/admin/data/user/M_User.java (2)
274-276: Null-safe deleted flag LGTM
Boolean.TRUE.equals(Deleted)is the right defensive default.
266-268: Preserve existing wrapper getter; add null-safe primitive helper
- Do not change the signature of
public Boolean isIsSupervisor()(keep it as-is to avoid breaking binary compatibility).- Add a new primitive accessor for null-safe checks:
public boolean isSupervisor() { return Boolean.TRUE.equals(IsSupervisor); }- (Optional) For standard JavaBean naming, you can also add
public Boolean getIsSupervisor() { return IsSupervisor; }- Deprecate
isIsSupervisor()if you plan to remove it in a future major release.- Manually verify that no external consumers (code or JSON contracts) rely on the old
isIsSupervisor()signature.src/main/java/com/iemr/admin/controller/employeemaster/EmployeeSignatureController.java (1)
103-106: Media type fallback LGTMGracefully defaults to
application/octet-streamon parse/null issues.
| */ | ||
| package com.iemr.admin.controller.employeemaster; | ||
|
|
||
| import java.net.URLEncoder; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t use URLEncoder for Content-Disposition
URLEncoder is for form/query encoding (space -> '+') and is incorrect for header params; it also requires Java 10+ for the Charset overload. Prefer Spring’s ContentDisposition builder, which emits both filename and RFC 5987 filename* correctly.
-import java.net.URLEncoder;Committable suggestion skipped: line range outside the PR's diff.
| String fileName = URLEncoder.encode(userSignID.getFileName(), StandardCharsets.UTF_8); | ||
|
|
||
| responseHeaders.set(HttpHeaders.CONTENT_DISPOSITION, | ||
| "attachment; filename=\"" + fileName + "\"; filename*=UTF-8''" + fileName); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Build standards-compliant Content-Disposition; avoid '+' for spaces
Manually URL-encoding breaks filename* (expects percent-encoding with spaces as %20) and may confuse some clients. Let Spring format it.
- String fileName = URLEncoder.encode(userSignID.getFileName(), StandardCharsets.UTF_8);
-
- responseHeaders.set(HttpHeaders.CONTENT_DISPOSITION,
- "attachment; filename=\"" + fileName + "\"; filename*=UTF-8''" + fileName);
+ org.springframework.http.ContentDisposition cd =
+ ContentDisposition.attachment()
+ .filename(userSignID.getFileName(), StandardCharsets.UTF_8)
+ .build();
+ responseHeaders.setContentDisposition(cd);If the project targets Java 8/11, this also removes reliance on URLEncoder.encode(String, Charset) (added post-8). Please confirm your baseline JDK.
🏁 Script executed:
#!/bin/bash
# Search for Java compiler version configuration in Maven/Gradle build files
rg -nP '<maven\\.compiler\\.source>' -n pom.xml
rg -nP '<maven\\.compiler\\.target>' -n pom.xml
rg -nP '<java\\.version>' -n pom.xml
rg -nP 'sourceCompatibility' -n .
rg -nP 'targetCompatibility' -n .
# Check usage of Spring's ContentDisposition and HttpHeaders.setContentDisposition
rg -nP 'ContentDisposition' -n src/main/java
rg -nP 'setContentDisposition' -n srcLength of output: 432
🏁 Script executed:
#!/bin/bash
# Extract parent section to determine Spring Boot version
sed -n '1,200p' pom.xml | sed -n '/<parent>/,/<\/parent>/p'
# Extract properties to determine java.version
sed -n '1,200p' pom.xml | sed -n '/<properties>/,/<\/properties>/p'
# Locate maven-compiler-plugin configuration
rg -nP '<artifactId>maven-compiler-plugin</artifactId>' -A5 -B5 -n pom.xml
# Check for spring-boot-starter-parent version explicit declaration
rg -nP '<artifactId>spring-boot-starter-parent</artifactId>' -A3 -B3 -n pom.xmlLength of output: 2702
Use Spring’s ContentDisposition builder for a standards-compliant Content-Disposition header
Manual URL-encoding with URLEncoder.encode(...) uses ‘+’ for spaces (invalid in filename*); use ContentDisposition.attachment().filename(..., UTF_8).build() and responseHeaders.setContentDisposition(...) to let Spring percent-encode correctly.
- String fileName = URLEncoder.encode(userSignID.getFileName(), StandardCharsets.UTF_8);
-
- responseHeaders.set(HttpHeaders.CONTENT_DISPOSITION,
- "attachment; filename=\"" + fileName + "\"; filename*=UTF-8''" + fileName);
+ ContentDisposition cd = ContentDisposition
+ .attachment()
+ .filename(userSignID.getFileName(), StandardCharsets.UTF_8)
+ .build();
+ responseHeaders.setContentDisposition(cd);Project uses Spring Boot 3.2.2 on Java 17, so the builder API is available.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String fileName = URLEncoder.encode(userSignID.getFileName(), StandardCharsets.UTF_8); | |
| responseHeaders.set(HttpHeaders.CONTENT_DISPOSITION, | |
| "attachment; filename=\"" + fileName + "\"; filename*=UTF-8''" + fileName); | |
| ContentDisposition cd = ContentDisposition | |
| .attachment() | |
| .filename(userSignID.getFileName(), StandardCharsets.UTF_8) | |
| .build(); | |
| responseHeaders.setContentDisposition(cd); |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/admin/controller/employeemaster/EmployeeSignatureController.java
around lines 96 to 99, replace the manual URLEncoder.encode(...) and header
string assembly with Spring's ContentDisposition builder: create a
ContentDisposition via
ContentDisposition.attachment().filename(userSignID.getFileName(),
StandardCharsets.UTF_8).build() and call
responseHeaders.setContentDisposition(...) (removing URLEncoder use and the
manual filename*= part) so Spring will percent-encode the filename correctly for
standards-compliant Content-Disposition.
* Compile error resolved * fix: cherry pic the #101 pr * fix: cherry pic the #101 pr * Main branch changes missed * fix: cherry pic the #103 pr * fix: cherry pic the #104 pr * fix: cherry pic the #104 pr * fix: cherry pick the #104 pr * fix code rabbit comments --------- Co-authored-by: Ravi Shanigarapu <ravi.shanigarapu@wipro.com>
* Bulk registration * Bulk registration * Bulk registration * add userName and password in Bulk registration * add userName and password in Bulk registration * remove unwanted line * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix code * Httpheader content disposition changed * Coderabbitai comments adrressed * Httpheader content disposition changed (#100) * Httpheader content disposition changed * Coderabbitai comments adrressed * Compile error resolved * fix code * Main branch changes missed (#102) * Feature/signaturerelease (#103) * Main branch changes missed * Signature file changed * Feature/signaturerelease (#104) * Main branch changes missed * Signature file changed * Created new endpoint for Active and DeActive Employee Signature * coderabbit comments addressed * fix:casesheet signature * fix:pom file change * API changes in Signature enhancement for Casesheet (#107) * fix:casesheet signature * fix:pom file change * fix: pom version * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix code * fix: amm-1927 send headers only if the request is from the allowed origin * fix: amm-1927 coderabbit fixes * Update regex handling for localhost URLs * Enhance regex pattern for URL matching * Cherry-pick health and version API enhancements to release-3.6.1 (#124) * feat(health,version): add health and version endponts * fix(health): add constant and remove duplicates * fix(health): avoid permanent DEGRADED from historical deadlocks * fix(health): Removed the unnecessary boolean literal * fix(health): Fixed the broken lock-wait detection * fix(health): avoid blocking DB I/O under write lock and restore interrupt flag * fix(health): add cancelFutures in healthservice * fix(health): close basic DB connection before advanced checks and remove shared-map race * fix: merge 3.6.1 to main --------- Co-authored-by: Saurav Mishra <saurav.mishra@bizbrolly.com> Co-authored-by: Sushant <77480199+sushant-bizbrolly@users.noreply.github.com> Co-authored-by: Saurav Mishra <80103738+SauravBizbRolly@users.noreply.github.com> Co-authored-by: Mithun James <drtechie@users.noreply.github.com> Co-authored-by: Ravi Shanigarapu <ravi.shanigarapu@wipro.com> Co-authored-by: ravishanigarapu <133210792+ravishanigarapu@users.noreply.github.com> Co-authored-by: vishwab1 <vishwanath@navadhiti.com> Co-authored-by: Vishwanath Balkur <118195001+vishwab1@users.noreply.github.com> Co-authored-by: SnehaRH <77656297+snehar-nd@users.noreply.github.com> Co-authored-by: Amoghavarsh <93114621+5Amogh@users.noreply.github.com> Co-authored-by: 5Amogh <amoghavarsh@navadhiti.com> Co-authored-by: KOPPIREDDY DURGA PRASAD <144464542+DurgaPrasad-54@users.noreply.github.com>
2 participants
📋 Description
JIRA ID:
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
Bug Fixes
Improvements