fix: Publishing Push Job Management API - 10 issues resolved#34874
Open
hassandotcms wants to merge 5 commits intomainfrom
Open
fix: Publishing Push Job Management API - 10 issues resolved#34874hassandotcms wants to merge 5 commits intomainfrom
hassandotcms wants to merge 5 commits intomainfrom
Conversation
This commit addresses all 10 issues identified in the Publishing Push publishing job management REST endpoints:
**HIGH Priority Fixes:**
1. DELETE /{bundleId} now returns 404 for non-existent bundles
2. POST /push/{bundleId} validates date format before bundle lookup
3. Added machine-readable enums for status query parameters
**MEDIUM Priority Fixes:**
4. Added allowableValues for deliveryStrategy parameter
5. Documented operation case-insensitivity vs deliveryStrategy case-sensitivity
6. Added date-time format and timezone requirements for date fields
7. Exposed filterKey in response views for API reproducibility
**LOW Priority Fixes:**
8. Documented purge async behavior and WebSocket notifications for API clients
9. Fixed bundleId examples to use correct ULID format
10. Added port range validation pattern for EndpointDetailView.port
**Additional Security & Validation Improvements:**
- Added bundleId format validation to prevent XSS attacks
- Fixed pagination validation to return 400 instead of silent clamping
- Enhanced OpenAPI documentation with proper schema constraints
Resolves #34862
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Freddy Montes <fmontes@users.noreply.github.com>
Open
4 tasks
- Restore dual-check (bundle + audit table) for DELETE endpoint to prevent orphaned audit records from becoming undeletable - Add DeliveryStrategyDeserializer to return clean error messages for invalid deliveryStrategy values without exposing Java class names - Cache parsed dates in push endpoint to avoid redundant double parsing - Update 3 pagination tests to expect 400 errors instead of clamping Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ue-34862-20260304-1534
…ue-34862-20260304-1534
dario-daza
reviewed
Mar 12, 2026
| } | ||
|
|
||
| // Validate bundleId format (prevent XSS and invalid characters) | ||
| if (!bundleId.matches("^[a-zA-Z0-9\\-_]+$")) { |
Contributor
There was a problem hiding this comment.
I reckon we can reuse this code over here in a separate method.
// Validate bundleId format (prevent XSS and invalid characters)
if (!bundleId.matches("^[a-zA-Z0-9\\-_]+$")) {
throw new BadRequestException("Invalid bundle ID format. Only alphanumeric characters, hyphens, and underscores are allowed.");
}
Contributor
There was a problem hiding this comment.
I agree, also we are repeating that regex in many places
dario-daza
approved these changes
Mar 12, 2026
| // 8. Execute operation based on type — parse only the dates required per operation | ||
| final String operation = form.getOperation().toLowerCase(); | ||
| // 9. Execute operation based on type (dates already parsed in step 4) | ||
| switch (operation) { |
Contributor
There was a problem hiding this comment.
The switch does not have a default case. Adding one improves usability
|
|
||
| // Validate bundleId format (prevent XSS and invalid characters) | ||
| if (!bundleId.matches("^[a-zA-Z0-9\\-_]+$")) { | ||
| throw new BadRequestException("Invalid bundle ID format. Only alphanumeric characters, hyphens, and underscores are allowed."); |
Contributor
There was a problem hiding this comment.
The regex is called via String.matches() in three places, which recompiles the pattern on every invocation:
if (!bundleId.matches("^[a-zA-Z0-9\-_]+$")) { ... }
Should be extracted to a compiled constant:
private static final Pattern BUNDLE_ID_PATTERN = Pattern.compile("^[a-zA-Z0-9_-]+$");
// then: if (!BUNDLE_ID_PATTERN.matcher(bundleId).matches()) { ... }
| final int validPerPage = Math.min(Math.max(1, perPage), MAX_PER_PAGE); | ||
| final int offset = (validPage - 1) * validPerPage; | ||
| // Validate pagination parameters | ||
| if (page < 1) { |
Member
There was a problem hiding this comment.
it says must be 1 or greater but not checking equals 1
| if (page < 1) { | ||
| throw new BadRequestException("Page must be 1 or greater (got: " + page + ")"); | ||
| } | ||
| if (perPage < 1 || perPage > MAX_PER_PAGE) { |
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
5 participants
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.
This PR addresses all 10 issues identified in the Publishing Push publishing job management REST endpoints:
HIGH Priority Fixes:
MEDIUM Priority Fixes:
4. Added allowableValues for deliveryStrategy parameter
5. Documented operation case-insensitivity vs deliveryStrategy case-sensitivity
6. Added date-time format and timezone requirements for date fields
7. Exposed filterKey in response views for API reproducibility
LOW Priority Fixes:
8. Documented purge async behavior and WebSocket notifications for API clients
9. Fixed bundleId examples to use correct ULID format
10. Added port range validation pattern for EndpointDetailView.port
Additional Security & Validation Improvements:
Resolves #34862
Generated with Claude Code