mlevkov commented on PR #3804: URL: https://github.com/apache/iggy/pull/3804#issuecomment-5444641080
Pushed `4a5b254`, and a correction to what I said above. I wrote that `configure_cors` panicking on bad CORS config "is consistent with bad config being fatal in this runtime". That is true of the runtime and I should have checked the rest of the repo before offering it as a rationale, because `core/server` already made the opposite call for the identically named field. `core/server/src/http.rs` rejects a misplaced wildcard with `IggyError::InvalidConfiguration` rather than letting `AllowOrigin::list` panic, and `core/server/config.toml` documents the position rule right above `allowed_origins`. So porting that guard is not a new design question, it is catching the connectors copy up. It is also the worse of the two: `api::init` runs at `main.rs:248`, after "All sources and sinks spawned", so the panic takes down a runtime whose connectors are already running. Until the guard is ported, the three shipped TOMLs now carry the sentence the server's config has had all along: ```toml # "*" is honored only as the first entry; anywhere else it panics at startup. ``` Happy to port `parse_cors_values` and the rejecting arm in a follow-up, or in this PR if you would rather it not ship with only a comment. One related note so the ordering does not bite later. The server trims each entry before comparing, and the two predicates here deliberately do not, because `configure_cors` does not either: `[" *"]` currently becomes a list entry no `Origin` matches, so it allows nothing and there is nothing to warn about. Trimming has to land in the predicate and the mapping together. Trimming only the predicate warns about a closed config, and trimming only the mapping opens one without a warning. That reasoning is now a comment on `allows_any_origin` so whoever ports the server's handling sees it. Also in this push: the `null` test was the fourth warning-emitting test to skip the pointer assertion the others carry, which is the same omission I fixed for the TLS test earlier in the round. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
