fix: correct rate limiting interval calculation in UIxMailEditor#373
Open
tobias-weiss-ai-xr wants to merge 7 commits intoAlinto:masterfrom
Open
fix: correct rate limiting interval calculation in UIxMailEditor#373tobias-weiss-ai-xr wants to merge 7 commits intoAlinto:masterfrom
tobias-weiss-ai-xr wants to merge 7 commits intoAlinto:masterfrom
Conversation
tobias-weiss-ai-xr
force-pushed
the
fix/rate-limit-interval-calculation
branch
2 times, most recently
from
24856af to
5c609d6
Compare
The rate limiting check was using messageSubmissionBlockInterval (block duration) instead of maximumSubmissionInterval (rate limit window) to determine if message submission should be blocked. Configuration example: SOGoMaximumMessageSubmissionCount: 5 messages SOGoMaximumRecipientCount: 100 recipients SOGoMaximumSubmissionInterval: 30 seconds (rate limit window) SOGoMessageSubmissionBlockInterval: 300 seconds (how long to block after violation) Previous incorrect behavior: Code checked: delta <= block_time (<= 300s) Result: 5th message blocked even if sent AFTER 30s had elapsed Correct behavior now: Code checks: delta < maximumSubmissionInterval (< 30s) Result: Only blocks if 5 messages sent WITHIN the 30s window This also simplifies the counter reset logic - reset when the submission interval has elapsed AND we're within limits.
tobias-weiss-ai-xr
force-pushed
the
fix/rate-limit-interval-calculation
branch
from
5c609d6 to
7976508
Compare
Contributor
|
Hello, Your fix was right but the user was no longer blocked for SOGoMessageSubmissionBlockInterval if they exceeds the 5 messages in 30seconds. They were free again after 30s. I've updated this PR, could you try it and tell me if this is okay ? |
…aders Fix critical bug where SOGo fails to quote display names containing special characters (commas, parentheses, brackets, etc.) when serializing email headers. This caused parsers to create bogus recipients and bounce emails. Root Cause: - The _quoteSpecials: method checked for special characters ANYWHERE in the address string (including email part), and since all emails contain @ and ., it was always triggering - It incorrectly extracted display names, assuming whitespace before < - It didn't detect already-formatted addresses (quoted or RFC 2047 encoded) Fix: - Complete rewrite of _quoteSpecials: method - Only checks display name (not email part) for special characters - Detects already-formatted addresses (quoted strings, encoded words) - Properly escapes backslashes and double quotes inside quoted strings - Follows RFC 5322 specification precisely Added helper methods: - _needsQuotingForPhrase: - RFC 5322-compliant special character check - _alreadyProperlyFormatted: - Detects quoted or encoded display names - _quoteAndEscape: - Proper quoting and escaping Test Coverage: - Added TestSOGoDraftObjectQuoteSpecials.m with 18 comprehensive tests - Covers all RFC 5322 special characters - Tests edge cases (nil, empty, whitespace, already-formatted) Fixes: Display names like "Lastname, Firstname (INFO)[MoreINFO]" now emit as: "Lastname, Firstname (INFO)[MoreINFO]" <user@example.com> Instead of: Lastname, Firstname (INFO)[MoreINFO] <user@example.com> Co-authored-by: TDD workflow See: docs/fix-email-header-serialization-bug.md for full analysis
Author
|
I am fine with that. |
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
2 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.
The rate limiting check was using messageSubmissionBlockInterval (block duration) instead of maximumSubmissionInterval (rate limit window) to determine if message submission should be blocked.
Configuration example:
SOGoMaximumMessageSubmissionCount: 5 messages
SOGoMaximumRecipientCount: 100 recipients
SOGoMaximumSubmissionInterval: 30 seconds (rate limit window)
SOGoMessageSubmissionBlockInterval: 300 seconds (how long to block after violation)
Previous incorrect behavior:
Code checked: delta <= block_time (<= 300s)
Result: 5th message blocked even if sent AFTER 30s had elapsed
Correct behavior now:
Code checks: delta < maximumSubmissionInterval (< 30s)
Result: Only blocks if 5 messages sent WITHIN the 30s window
This also simplifies the counter reset logic - reset when the submission interval has elapsed AND we're within limits.