[FIX] 문서 NUL 문자 정제 및 Discord 알림 길이 제한 - #199
Merged
Merged
Conversation
1 task
|
Warning Review limit reached
More reviews will be available in 51 minutes and 6 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
요약이 PR은 두 가지 독립적인 입력 데이터 정제 기능을 추가합니다: 문서 생성 요청에서 null 문자를 제거하는 기능과 Discord 알림 메시지를 2000자로 제한하는 기능입니다. 변경 사항문서 요청 null 문자 제거
Discord 메시지 길이 제한
예상 검토 노력🎯 3 (보통) | ⏱️ ~20분 제안 라벨
시
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
Author
📝 Code Coverage
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
ssd-api/src/main/java/or/hyu/ssd/api/document/request/RequestStringSanitizer.java (1)
8-13: 💤 Low value문자 리터럴과 문자열 리터럴의 일관성 개선 권장
Line 9에서는
'\u0000'(char 리터럴)을 사용하고 Line 12에서는"\u0000"(String 리터럴)을 사용하고 있습니다. 동작에는 문제가 없지만, 가독성을 위해 동일한 표현 방식을 사용하는 것이 좋습니다.♻️ 일관성 개선 제안
static String stripNullChar(String value) { - if (value == null || value.indexOf('\u0000') < 0) { + if (value == null || !value.contains("\u0000")) { return value; } return value.replace("\u0000", ""); }🤖 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 `@ssd-api/src/main/java/or/hyu/ssd/api/document/request/RequestStringSanitizer.java` around lines 8 - 13, The stripNullChar method uses a char literal '\u0000' in value.indexOf(...) but a String literal "\u0000" in value.replace(...); make these consistent (prefer using the String literal "\u0000" for both calls or the char form for both) by updating the indexOf call or the replace call in stripNullChar so both use the same literal form to improve readability and consistency.ssd-external/src/test/java/or/hyu/ssd/external/alert/discord/DiscordWebhookNotifierTest.java (2)
12-40: ⚡ Quick winnull 입력 처리에 대한 테스트 케이스 추가를 권장합니다.
현재 구현부(DiscordWebhookNotifier line 137)에서
null입력 시null을 반환하는데, 이는escapeJson에서 NPE를 유발할 수 있습니다.null입력에 대한 테스트를 추가하면 이런 이슈를 조기에 발견할 수 있습니다.♻️ null 테스트 추가 제안
`@Test` `@DisplayName`("디스코드 본문이 null이면 빈 문자열을 반환한다") void returnEmptyStringWhenContentIsNull() throws Exception { DiscordWebhookNotifier notifier = new DiscordWebhookNotifier(new DiscordProperties(), new MockEnvironment()); Method truncateMethod = DiscordWebhookNotifier.class.getDeclaredMethod("truncateForDiscord", String.class); truncateMethod.setAccessible(true); String truncated = (String) truncateMethod.invoke(notifier, (String) null); assertThat(truncated).isEqualTo(""); }🤖 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 `@ssd-external/src/test/java/or/hyu/ssd/external/alert/discord/DiscordWebhookNotifierTest.java` around lines 12 - 40, The truncateForDiscord method in DiscordWebhookNotifier currently returns null for null input which can cause a NullPointerException later in escapeJson; change truncateForDiscord to handle null by returning an empty string instead, and add a unit test (as suggested) that invokes DiscordWebhookNotifier.truncateForDiscord with a null argument and asserts it returns "" to prevent NPEs in escapeJson and downstream logic.
14-26: ⚡ Quick win테스트에 내용 검증을 추가하면 더 견고해집니다.
현재 테스트는 길이와 접미사 존재 여부만 확인합니다. 잘린 메시지의 앞부분이 원본 내용을 올바르게 보존하는지도 검증하면 truncation 로직의 정확성을 더 확실히 보장할 수 있습니다.
♻️ 내용 검증 추가 제안
String longMessage = "a".repeat(2100); String truncated = (String) truncateMethod.invoke(notifier, longMessage); assertThat(truncated.length()).isLessThanOrEqualTo(2000); assertThat(truncated).endsWith("\n...(truncated)"); +assertThat(truncated).startsWith(longMessage.substring(0, 1983));🤖 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 `@ssd-external/src/test/java/or/hyu/ssd/external/alert/discord/DiscordWebhookNotifierTest.java` around lines 14 - 26, Update the test truncateMessageWhenContentTooLong to also verify that the truncated string preserves the original beginning content: after invoking DiscordWebhookNotifier.truncateForDiscord, assert that the returned value starts with the original message's first N characters (e.g., compare substring(0, expectedPrefixLen) of the original longMessage with substring(0, expectedPrefixLen) of truncated) in addition to the existing length and suffix checks; locate the test method truncateMessageWhenContentTooLong in DiscordWebhookNotifierTest and the truncateForDiscord method in DiscordWebhookNotifier to determine the appropriate prefix length to assert.
🤖 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
`@ssd-api/src/main/java/or/hyu/ssd/api/document/request/CreateDocumentBlockRequest.java`:
- Around line 40-47: The validation currently runs before NUL characters are
stripped so ROLE_PATTERN rejects inputs like "\u0000##"; update the
CreateDocumentBlockRequest validation to sanitize role before validating by
calling RequestStringSanitizer.stripNullChar(role) inside
isValidBlockStructure() (or the validation method) and use that sanitizedRole
for ROLE_PATTERN.matcher(...).matches() and other checks; ensure resolvedType()
and other branching logic remain the same and keep toCommand() unchanged (it can
still strip defensively), so validation operates on the cleaned value rather
than the raw input.
In
`@ssd-api/src/test/java/or/hyu/ssd/api/document/request/CreateDocumentRequestValidationTest.java`:
- Around line 67-88: The test currently calls CreateDocumentRequest.toCommand()
directly which bypasses bean validation; add a new test that submits the same
request to the validation step (use VALIDATOR.validate(request)) to assert
whether violations are empty, referencing CreateDocumentRequest and
CreateDocumentBlockRequest; then modify the validation logic in
CreateDocumentBlockRequest.isValidBlockStructure() (or adjust ROLE_PATTERN) to
either strip NUL characters before applying ROLE_PATTERN or update ROLE_PATTERN
to allow leading/trailing NULs so the request that contains "\u0000##" will pass
validation and the downstream toCommand() sanitization remains effective.
In
`@ssd-external/src/main/java/or/hyu/ssd/external/alert/discord/DiscordWebhookNotifier.java`:
- Around line 136-146: The truncateForDiscord method currently returns null when
content is null which later causes a NullPointerException in buildPayload ->
escapeJson (due to .replace on null); update truncateForDiscord so it never
returns null (return an empty string or other safe default) and still apply
truncation logic (use DISCORD_CONTENT_LIMIT and TRUNCATION_SUFFIX as before),
ensuring buildPayload and escapeJson always receive non-null input.
---
Nitpick comments:
In
`@ssd-api/src/main/java/or/hyu/ssd/api/document/request/RequestStringSanitizer.java`:
- Around line 8-13: The stripNullChar method uses a char literal '\u0000' in
value.indexOf(...) but a String literal "\u0000" in value.replace(...); make
these consistent (prefer using the String literal "\u0000" for both calls or the
char form for both) by updating the indexOf call or the replace call in
stripNullChar so both use the same literal form to improve readability and
consistency.
In
`@ssd-external/src/test/java/or/hyu/ssd/external/alert/discord/DiscordWebhookNotifierTest.java`:
- Around line 12-40: The truncateForDiscord method in DiscordWebhookNotifier
currently returns null for null input which can cause a NullPointerException
later in escapeJson; change truncateForDiscord to handle null by returning an
empty string instead, and add a unit test (as suggested) that invokes
DiscordWebhookNotifier.truncateForDiscord with a null argument and asserts it
returns "" to prevent NPEs in escapeJson and downstream logic.
- Around line 14-26: Update the test truncateMessageWhenContentTooLong to also
verify that the truncated string preserves the original beginning content: after
invoking DiscordWebhookNotifier.truncateForDiscord, assert that the returned
value starts with the original message's first N characters (e.g., compare
substring(0, expectedPrefixLen) of the original longMessage with substring(0,
expectedPrefixLen) of truncated) in addition to the existing length and suffix
checks; locate the test method truncateMessageWhenContentTooLong in
DiscordWebhookNotifierTest and the truncateForDiscord method in
DiscordWebhookNotifier to determine the appropriate prefix length to assert.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed350f31-1c23-4b64-ae9f-6199d43fc6ba
📒 Files selected for processing (7)
ssd-api/src/main/java/or/hyu/ssd/api/document/request/CreateDocumentBlockRequest.javassd-api/src/main/java/or/hyu/ssd/api/document/request/CreateDocumentRequest.javassd-api/src/main/java/or/hyu/ssd/api/document/request/RequestStringSanitizer.javassd-api/src/main/java/or/hyu/ssd/api/document/request/UpdateDocumentRequest.javassd-api/src/test/java/or/hyu/ssd/api/document/request/CreateDocumentRequestValidationTest.javassd-external/src/main/java/or/hyu/ssd/external/alert/discord/DiscordWebhookNotifier.javassd-external/src/test/java/or/hyu/ssd/external/alert/discord/DiscordWebhookNotifierTest.java
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
📣 Related Issue
📝 Summary
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항
테스트