[
https://issues.apache.org/jira/browse/TIKA-5025?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18138128#comment-18138128
]
ASF GitHub Bot commented on TIKA-5025:
--------------------------------------
tballison commented on PR #3519:
URL: https://github.com/apache/tika/pull/3519#issuecomment-6639221544
Only nits from by :robot: . Let me know what you think
```
Edge cases (worth fixing):
1. An option silently cancels a setting in Docker. The Docker entrypoint
always passes -h 0.0.0.0. So
TIKA_SERVER_SETTING_HOST=127.0.0.1 is logged as "set by
TIKA_SERVER_SETTING_HOST", and then -h replaces it with
no message. My probe got host=0.0.0.0. The same happens with PORT
against -p, ID against -i and METRICS_PORT
against --metricsPort. A user who tries to narrow the bind address gets
the wider one. Cheapest fix: fail at
startup, or at least warn, when an option and a setting name the same
field.
2. Text values are mangled. The PR converts any value that looks like a
number into a number before binding, so
TIKA_SERVER_SETTING_ID=007 becomes id 7, and the same would happen with
cors or host. Passing every value as a
plain string fixes this: Jackson already converts strings to long and
boolean. My probes showed it rejects "1.5"
for an int field and accepts "TRUE". That also removes the valueNode
helper and its 18-digit regex.
3. Two variables can name the same field. ALLOW_PIPES and ALLOW__PIPES
both become allowPipes. The first one read
wins, and the environment's order isn't defined, so which value applies
is arbitrary and nothing says so. Fix:
refuse two sources for the same field at the same level.
4. TIKA_SERVER_SETTING_PRESETS is accepted and then thrown away. Startup
succeeds and the presets list is empty. It
should be refused with a pointer to TIKA_PRESETS, the way configPath is
already refused with a pointer to -c.
Minor clean-ups:
- The PR title and the first commit message still say TIKA_SERVER_<FIELD>.
A squash merge would put the wrong
variable name in the history.
- The server docs open with "Every field can also be set", then exclude
tlsConfig and endpoints a few lines later.
Saying "every field except tlsConfig and endpoints" up front would be
clearer.
```
> tika-server: set the request limits from the environment and the command line
> -----------------------------------------------------------------------------
>
> Key: TIKA-5025
> URL: https://issues.apache.org/jira/browse/TIKA-5025
> Project: Tika
> Issue Type: Improvement
> Reporter: Dominik Schmidt
> Priority: Major
>
> Since TIKA-4988 the Docker image runs a thumbnail server with nothing but -e
> TIKA_PRESETS=thumbnail. The first file over 1 GiB brings the config file
> back: maxRequestSizeBytes (TIKA-4991, default 1073741824) lives in the server
> block of the JSON config and can only be changed by mounting a file and
> passing -c. The same goes for its siblings maxInFlightRequestBytes,
> maxRequestReadMillis, maxResponseWriteMillis and maxQueuePauseMillis.
> Measured on 4.2.1-SNAPSHOT with a 1.46 GB MPEG-4 audiobook: PUT
> /unpack/preset/thumbnail answers 413 "Request body exceeds
> maxRequestSizeBytes" from the declared Content-Length alone; with {"server":
> {"maxRequestSizeBytes": 4294967296}, "presets": {"thumbnail": true}} the
> cover comes back in 4 s.
> Proposal: every setting of the server block can be given as
> TIKA_SERVER_<UPPER_SNAKE_CASE> in the environment, e.g.
> TIKA_SERVER_MAX_REQUEST_SIZE_BYTES=4294967296, and as --server-<kebab-case>
> <value> on the command line, following TIKA-4988: both are merged into the
> startup config before the front end and the forked workers load it, so the
> forks keep receiving the resolved config over stdin and never read the
> environment; command line and environment win over the file; an unknown key
> fails startup; the startup log says where a value came from. Sizes accept the
> plain byte count, suffixes (4g) would be nice but are a separate decision.
> Also: the 413 body should name the limit and where to raise it, as the preset
> 404 does, so an operator reads the answer instead of the server log.
> Docs: the server page's limits section and the Docker page get the env form
> next to the config form.
> Related: TIKA-4988 (TIKA_PRESETS), TIKA-4991 (the limits), TIKA-4909 (env
> interpolation inside a config file, which still needs the file).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
