fix: atomic contact-form rate limiting, JavaChat crawler metadata, and new-tab menu links - #162
Conversation
Concurrent submissions from one IP could each observe the counter below the hourly allowance and all send before any increment landed, exceeding the advertised per-IP limit by an arbitrary burst. Reserve one slot with a single atomic increment before sending and release it when the reservation exceeds the allowance or delivery fails, so the check and increment cannot interleave. Flagged by Codex review on PR #161.
The ${SPRING_MAIL_HOST:...}/${SPRING_MAIL_PORT:...} placeholders declared a
new env-var-driven settings contract; repository policy [EV1c] keeps non-secret
defaults in Spring property files. Plain defaults suffice because Spring
relaxed binding already maps SPRING_MAIL_HOST/SPRING_MAIL_PORT onto
spring.mail.host/spring.mail.port, the same mechanism the credentials use.
Flagged by Codex review on PR #161.
SeoController still emitted the old "Java Chat" titles, descriptions, and JSON-LD name, so crawlers and social-preview clients that never run the SPA saw stale branding. Mirror the frontend pageMetadata catalog so the HTTP boundary and the SPA emit identical metadata for every routed path. Flagged by Codex review on PR #161.
The unconditional preventDefault() suppressed the anchor's new-tab behavior, so Command/Ctrl-clicking Privacy or Contact replaced the current SPA view instead of opening a new tab. Intercept only unmodified primary-button clicks so the real hrefs retain standard browser navigation semantics. Flagged by Codex review on PR #161.
SpotBugs THROWS_METHOD_THROWS_RUNTIMEEXCEPTION rejected rethrowing a caught RuntimeException; Spring's mail sender only raises MailException subtypes, so the catch narrows to the typed delivery failure without changing behavior.
The app uses only platform TLS, which is exempt from export compliance review. Declaring ITSAppUsesNonExemptEncryption=false lets App Store Connect and TestFlight process builds without the manual encryption questionnaire on every upload.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes update modified-click handling in the header menu, improve contact submission rate-limit recovery, revise JavaChat SEO metadata, set SMTP defaults, and add an iOS encryption declaration. ChangesFrontend navigation
Contact delivery
SEO metadata
iOS configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ContactSubmissionUseCase
participant RateLimitCounter
participant SMTPServer
ContactSubmissionUseCase->>RateLimitCounter: Reserve submission slot
RateLimitCounter-->>ContactSubmissionUseCase: Return reservation result
ContactSubmissionUseCase->>SMTPServer: Deliver contact mail
SMTPServer-->>ContactSubmissionUseCase: Return delivery result
ContactSubmissionUseCase->>RateLimitCounter: Release failed reservation
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
coderabbitai
Bot
added
bug
There was a problem hiding this comment.
Pull request overview
This PR applies targeted follow-up fixes across backend, frontend, and iOS packaging: it hardens the contact form’s per-IP rate limit under concurrency, aligns server-rendered SEO metadata with the JavaChat rebrand (for crawlers/social previews), restores native new-tab link behavior in the header menu, and adjusts mail/iOS configuration to match stated policies and App Store submission requirements.
Changes:
- Make contact-form rate limiting atomic by reserving the per-IP slot via an early atomic increment and releasing on over-limit or mail delivery failure (
ContactSubmissionUseCase.submit). - Update backend-rendered SEO titles/descriptions + JSON-LD app name to “JavaChat”, with test updates to match (
SeoController,SeoControllerTest). - Preserve native browser behavior for modified/non-primary clicks on Privacy/Contact menu links (Cmd/Ctrl-click new tab), with a regression test (
HeaderMenu.svelte,HeaderMenu.test.ts).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/williamcallahan/javachat/application/contact/ContactSubmissionUseCase.java | Reserves rate-limit capacity atomically before sending email; decrements on limit breach or mail send failure. |
| src/main/java/com/williamcallahan/javachat/web/SeoController.java | Updates server-rendered SPA SEO metadata and JSON-LD application name to JavaChat. |
| src/test/java/com/williamcallahan/javachat/web/SeoControllerTest.java | Updates assertions to the new JavaChat titles/descriptions. |
| src/main/resources/application.properties | Replaces SPRING_MAIL_HOST/PORT placeholders with plain defaults, relying on standard Spring override mechanisms. |
| frontend/src/lib/components/HeaderMenu.svelte | Skips SPA interception for modified/non-primary clicks to retain native anchor semantics. |
| frontend/src/lib/components/HeaderMenu.test.ts | Adds coverage ensuring modified-click events do not call preventDefault() and do not switch views. |
| mobile/iosApp/JavaChat/Info.plist | Declares ITSAppUsesNonExemptEncryption=false to avoid repeated export-compliance prompts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2245b083b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Non-secret defaults live here per policy; deployments override host/port through Spring | ||
| # relaxed binding (SPRING_MAIL_HOST / SPRING_MAIL_PORT) without placeholders in this file. |
There was a problem hiding this comment.
The new “per policy” explanation restates the repository directive governing where non-secret defaults and deployment overrides belong, turning application.properties into a second policy source. Keep this comment limited to runtime behavior or remove it, leaving the policy exclusively in AGENTS.md as required.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
![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
🧹 Nitpick comments (3)
frontend/src/lib/components/HeaderMenu.test.ts (1)
117-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the other modified-click branches.
The handler groups
ctrlKey,shiftKey,altKey, and non-primary buttons withmetaKey, but this test covers onlymetaKey. Add parameterized cases for the remaining branches so future changes cannot break those paths silently.🤖 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 `@frontend/src/lib/components/HeaderMenu.test.ts` around lines 117 - 127, Add parameterized cases to the modified-click test in HeaderMenu, covering ctrlKey, shiftKey, altKey, and non-primary button inputs alongside the existing metaKey case. For each case, verify the click remains unprevented and Contact does not receive aria-current.src/main/java/com/williamcallahan/javachat/web/SeoController.java (1)
63-91: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover every changed SEO field in the tests.
SeoController.initMetadata()changes the root and chat descriptions at Lines 64 and 72. Line 177 changes the JSON-LD application name.SeoControllerTestdoes not assert those descriptions orjava-chat-structured-data. Add assertions for these values. The suppliedsrc/test/resources/static/index.htmlfixture also needs the structured-data element; otherwiseupdateJsonLdcan skip the update while the test still passes.Also applies to: 177-177
🤖 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 `@src/main/java/com/williamcallahan/javachat/web/SeoController.java` around lines 63 - 91, Update SeoControllerTest to assert the changed root and /chat metadata descriptions and the java-chat-structured-data JSON-LD application name produced by SeoController.initMetadata(). Add the structured-data element to the static index.html test fixture so updateJsonLd exercises the update path instead of silently skipping it.src/main/java/com/williamcallahan/javachat/application/contact/ContactSubmissionUseCase.java (1)
78-90: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a concurrency regression test for the reservation lifecycle.
Run more than
MAX_ACCEPTED_SUBMISSIONS_PER_IPsubmissions for one IP concurrently. Assert that only the allowed number reachesJavaMailSender.sendand the excess request raisesContactRateLimitExceededException. Then make one send throwMailExceptionand verify that the next submission can reserve the released slot. A focused test gives this race-sensitive change an executable contract. Spring’sJavaMailSender.sendcontract declaresMailExceptionfor send failures. (docs.spring.io)🤖 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 `@src/main/java/com/williamcallahan/javachat/application/contact/ContactSubmissionUseCase.java` around lines 78 - 90, Add a focused concurrency regression test for the reservation lifecycle in ContactSubmissionUseCase: concurrently submit more than MAX_ACCEPTED_SUBMISSIONS_PER_IP for the same IP, assert only the allowed submissions invoke JavaMailSender.send and excess calls raise ContactRateLimitExceededException, then simulate one send throwing MailException and verify the released slot can be reserved by the next submission.Source: MCP tools
🤖 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 `@src/main/resources/application.properties`:
- Around line 117-118: Add finite SMTP connectiontimeout, timeout, and
writetimeout settings under spring.mail.properties.mail.smtp in
application.properties, using values appropriate for bounded synchronous
JavaMailSender.send execution. Keep the existing spring.mail.host and
spring.mail.port settings unchanged.
In `@src/test/java/com/williamcallahan/javachat/web/SeoControllerTest.java`:
- Line 42: Move the SEO titles and descriptions currently defined in
SeoController.initMetadata() into one typed canonical metadata catalog, then
have both SeoController and SeoControllerTest project their expected values from
that catalog. Remove duplicated literal metadata assertions while retaining
independent assertions for rendered fields and URLs, and ensure the test binds
or imports the catalog rather than mirroring its values.
---
Nitpick comments:
In `@frontend/src/lib/components/HeaderMenu.test.ts`:
- Around line 117-127: Add parameterized cases to the modified-click test in
HeaderMenu, covering ctrlKey, shiftKey, altKey, and non-primary button inputs
alongside the existing metaKey case. For each case, verify the click remains
unprevented and Contact does not receive aria-current.
In
`@src/main/java/com/williamcallahan/javachat/application/contact/ContactSubmissionUseCase.java`:
- Around line 78-90: Add a focused concurrency regression test for the
reservation lifecycle in ContactSubmissionUseCase: concurrently submit more than
MAX_ACCEPTED_SUBMISSIONS_PER_IP for the same IP, assert only the allowed
submissions invoke JavaMailSender.send and excess calls raise
ContactRateLimitExceededException, then simulate one send throwing MailException
and verify the released slot can be reserved by the next submission.
In `@src/main/java/com/williamcallahan/javachat/web/SeoController.java`:
- Around line 63-91: Update SeoControllerTest to assert the changed root and
/chat metadata descriptions and the java-chat-structured-data JSON-LD
application name produced by SeoController.initMetadata(). Add the
structured-data element to the static index.html test fixture so updateJsonLd
exercises the update path instead of silently skipping it.
🪄 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 Plus
Run ID: 053943f3-8f7e-489d-b350-075653819988
📒 Files selected for processing (7)
frontend/src/lib/components/HeaderMenu.sveltefrontend/src/lib/components/HeaderMenu.test.tsmobile/iosApp/JavaChat/Info.plistsrc/main/java/com/williamcallahan/javachat/application/contact/ContactSubmissionUseCase.javasrc/main/java/com/williamcallahan/javachat/web/SeoController.javasrc/main/resources/application.propertiessrc/test/java/com/williamcallahan/javachat/web/SeoControllerTest.java
| spring.mail.host=localhost | ||
| spring.mail.port=587 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
mapfile -t application_configs < <(fd -t f 'application.*\.(properties|yml|yaml)$')
rg -n 'mail\.smtp\.(connectiontimeout|timeout|writetimeout)|spring\.mail\.(host|port)' \
"${application_configs[@]}"Repository: WilliamAGH/java-chat
Length of output: 210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== application config files =="
git ls-files | grep -E '(^|/)application\..*\.(properties|yml|yaml)$' || true
echo
echo "== mail settings in repository =="
rg -n 'mail\.smtp\.(connectiontimeout|timeout|writetimeout)|spring\.mail\.(host|port|auth|starttls)|mail\.smtp\.(connectiontimeout|timeout|writetimeout)' . --glob 'src/main/resources/application*.properties' --glob 'src/main/resources/application*.yml' --glob 'src/main/resources/application*.yaml' || true
echo
echo "== application.properties lines 1-160 =="
sed -n '1,160p' src/main/resources/application.properties
echo
echo "== relevant source snippets =="
sed -n '50,95p' src/main/java/com/williamcallahan/javachat/web/ContactController.java
sed -n '70,100p' src/main/java/com/williamcallahan/javachat/ContactSubmissionUseCase.javaRepository: WilliamAGH/java-chat
Length of output: 11627
🌐 Web query:
Spring Boot mail SMTP timeout properties default infinite connectiontimeout timeout writetimeout
💡 Result:
In Spring Boot, the default timeout values for SMTP mail operations are infinite [1][2][3][4]. This means that if an SMTP server is unresponsive, your application's threads can remain blocked indefinitely [1][5]. To prevent this, you should explicitly configure the following properties in your application configuration (e.g., application.properties or application.yml) [1][5]: - mail.smtp.connectiontimeout: The socket connection timeout in milliseconds [6]. - mail.smtp.timeout: The socket I/O timeout in milliseconds [6]. - mail.smtp.writetimeout: The socket write timeout in milliseconds. Example Configuration (application.properties): spring.mail.properties.mail.smtp.connectiontimeout=5000 spring.mail.properties.mail.smtp.timeout=3000 spring.mail.properties.mail.smtp.writetimeout=5000 Example Configuration (application.yml): spring: mail: properties: "[mail.smtp.connectiontimeout]": 5000 "[mail.smtp.timeout]": 3000 "[mail.smtp.writetimeout]": 5000 Note that the key used for connection timeout must be mail.smtp.connectiontimeout, not mail.smtp.connecttimeout [7]. These properties are passed to the underlying JavaMail session [1][8].
Citations:
- 1: https://docs.spring.io/spring-boot/reference/io/email.html
- 2: https://docs.spring.io/spring-boot/docs/1.5.18.RELEASE/reference/html/boot-features-email.html
- 3: https://docs.spring.io/spring-boot/docs/2.0.0.M7/reference/html/boot-features-email.html
- 4: https://docs.spring.io/spring-boot/docs/2.1.18.RELEASE/reference/html/boot-features-email.html
- 5: https://github.com/spring-projects/spring-boot/blob/v3.3.13/spring-boot-project/spring-boot-docs/src/docs/antora/modules/reference/pages/io/email.adoc
- 6: https://connector.sourceforge.net/doc-files/Properties.html
- 7: Invalid mail.smtp.connecttimeout key in the documentation spring-projects/spring-boot#9082
- 8: https://docs.spring.io/spring-boot/docs/3.0.13/reference/html/application-properties.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== use case candidates =="
fd -t f 'ContactSubmissionUseCase.java|ContactSubmissionUseCase.*' . || true
echo
echo "== submit/send/mail references =="
rg -n 'JavaMailSender|send\(.*contact|contactSubmission|rate limit|capacity|reservation|LocalDataStore|ContactSubmissionUseCase' src/main/java || true
echo
echo "== files containing submit method =="
rg -n 'void submit|public .*submit|ContactSubmissionUseCase' src/main/java || trueRepository: WilliamAGH/java-chat
Length of output: 12693
Set finite SMTP timeouts for the synchronous mail send.
The contact submission flow blocks on JavaMailSender.send after accepting the client rate-limit admission. application.properties supplies spring.mail.host and spring.mail.port, but no SMTP connectiontimeout, timeout, or writetimeout. Add finite mail time-outs under spring.mail.properties.mail.smtp so an unresponsive SMTP server cannot hold request threads and per-IP admission indefinitely.
🤖 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 `@src/main/resources/application.properties` around lines 117 - 118, Add finite
SMTP connectiontimeout, timeout, and writetimeout settings under
spring.mail.properties.mail.smtp in application.properties, using values
appropriate for bounded synchronous JavaMailSender.send execution. Keep the
existing spring.mail.host and spring.mail.port settings unchanged.
Source: MCP tools
| Document htmlDocument = Jsoup.parse(mvcOutcome.getResponse().getContentAsString()); | ||
|
|
||
| assertEquals("Java Chat - AI-Powered Java Learning With Citations", htmlDocument.title()); | ||
| assertEquals("JavaChat - AI Learning", htmlDocument.title()); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Keep SEO metadata in one canonical owner.
These assertions repeat the titles and descriptions defined in SeoController.initMetadata(). If both files change together, the test can pass while the canonical metadata is wrong. Move the page metadata to one typed catalog and have SeoController and SeoControllerTest project from it. Keep independent assertions for rendered fields and URLs.
As per coding guidelines, tests must bind/import/project the canonical owner instead of restating it (No Mirrors).
Also applies to: 55-94
🤖 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 `@src/test/java/com/williamcallahan/javachat/web/SeoControllerTest.java` at
line 42, Move the SEO titles and descriptions currently defined in
SeoController.initMetadata() into one typed canonical metadata catalog, then
have both SeoController and SeoControllerTest project their expected values from
that catalog. Remove duplicated literal metadata assertions while retaining
independent assertions for rendered fields and URLs, and ensure the test binds
or imports the catalog rather than mirroring its values.
Source: Coding guidelines
Summary
Follow-up repairs from the Codex review of #161: the contact form's per-IP rate limit can no longer be exceeded by concurrent bursts, crawlers and social previews now see the rebranded JavaChat metadata, and header menu links keep standard new-tab browser behavior — plus an iOS build setting that removes the manual encryption questionnaire from every App Store upload.
Changes
Bug Fixes
ContactSubmissionUseCase.submit)SeoControllerstill emitted the old titles, descriptions, and JSON-LD name, so clients that never run the SPA (crawlers, social previews) saw pre-rebrand metadata; server-rendered metadata now mirrors the frontend pageMetadata catalog for every routed path (SeoController.initMetadata,SeoControllerTest)preventDefault()suppressed the anchors' native new-tab behavior for Privacy and Contact; only unmodified primary-button clicks are intercepted now (HeaderMenu.svelte.navigateToSiteLink)Configuration
${SPRING_MAIL_HOST:...}/${SPRING_MAIL_PORT:...}placeholders declared a new env-var-driven settings contract; host and port are now plain property-file defaults per [EV1c], with deployment override unchanged via Spring relaxed binding — the same mechanism the SMTP credentials use (application.properties)Build
ITSAppUsesNonExemptEncryption=false, since it uses only platform TLS, so App Store Connect and TestFlight process builds without the manual encryption questionnaire (mobile/iosApp/JavaChat/Info.plist)Breaking Changes
None
Related
Follow-up to #161 (merged) addressing its Codex review threads.