Repository navigation
core: Add common apr_off_t config parser - #829
Open
arturobernalg wants to merge 1 commit into
Open
arturobernalg wants to merge 1 commit into
arturobernalg wants to merge 1 commit into
Conversation
Add ap_off_parameter_parse(), a helper to parse a configuration value as an apr_off_t, which fails on trailing characters and on overflow. The range of the value remains up to the directive. Use it for configuration directives which parsed such values in different ways, some of them accepting trailing characters or no checks at all: LimitRequestBody (unchanged behavior), ScriptLogLength, ScriptLogBuffer, DavQuota, BufferSize, AuthFormSize, and the Cache*FileSize, Cache*ReadSize, Cache*ReadTime and CacheSocache*Time directives.
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
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.
Follow-up to the parsing discussion in #825.
Adds a common helper,
ap_off_parameter_parse(), for converting configuration input to anapr_off_t, analogous to the existing timeout parser. It accepts a whole decimal number (asapr_strtoff()does) and fails on trailing characters, an empty value and overflow. It returnsAPR_EINVALfor a malformed number andAPR_ERANGEfor overflow, and only sets the value on success. The range of the value stays with each directive.It converts the existing callers where the value is an
apr_off_tor fits in one, and keeps each directive's own range checks (narrowing tolong,intorapr_size_tis checked before the cast):LimitRequestBody(no change in behavior or messages, checked by comparing the accepted values before and after)CacheMinFileSize,CacheMaxFileSize,CacheReadSize,CacheReadTime(mod_cache_disk)CacheSocacheMaxSize,CacheSocacheMaxTime,CacheSocacheMinTime,CacheSocacheReadSize,CacheSocacheReadTimeBufferSize,AuthFormSizeDavQuotaScriptLogLength,ScriptLogBuffer(mod_cgi and mod_cgid)This also makes malformed values such as
123abcfail at startup consistently instead of being partially accepted. Most of these directives usedapr_strtoff()without checking the end of the number, andDavQuota,ScriptLogLengthandScriptLogBufferusedatol()/atoi(), soabcmeant 0. Negative values ofDavQuota(they also took the place of the Off/None values),ScriptLogLengthandScriptLogBufferare now refused.LimitXMLRequestBodyis left to #825, and the directives with units (e.g.SubstituteMaxLineLength) are not touched.The new function is exported API, so the MMN minor is bumped.
Tested with
httpd -tagainst trunk and the patch for every directive above, and with new pyhttpd tests (core/cache/cgid, DavQuota, BufferSize). I also ran the pytest_suite cache, cgi, buffer, dav and limits tests, where nothing changed. The new tests were not run on Windows. mod_auth_form is only checked by that comparison, and the mod_cgi variant was only compiled.(see
SECURITY.mdandhttps://www.apache.org/security/#reporting-a-vulnerability).
APLOGNO()tag;numbers are assigned by a committer at merge time (see
docs/log-message-tags/README).(No new log messages.)
changes-entries/*.txtfile is included,following the template in
README.CHANGES(not needed otherwise).test/modules/xxxdirectory matching themodules/xxxmodule source,or
test/modules/corefor core server changes. On Unix, verify thesepass locally with e.g.
make check-pytest PYTEST_DIRS=test/modules/xxx.