SEZ9 commented on PR #12293: URL: https://github.com/apache/seatunnel/pull/12293#issuecomment-6356466852
Thanks for the update and for pushing a02d87a6b18473e304e679a67127e9947ac80807 (and the GHA run 39829657690). The status table in your comment appears to be cut off after item 6, and the numbering doesn't line up one-to-one with the findings I raised, so I can't yet confirm which of them are closed. From what is visible: - Item 3 (trust-boundary wording on `hadoop_adls_properties` in `ADLSFileBaseOptions.java` and the four doc pages) looks related to **PR12293-F8**. Does the wording now reflect that the protection is a prefix denylist, or did you also tighten the denylist to cover class-loading / token-provider ABFS keys? Either is fine, but the docs and the validator should agree. - Item 5 touches `ADLSHadoopConf.java` and `ADLSRuntimeCompatibility.java`, but it describes removing `setSchema` and narrowing test-helper visibility; I don't see it addressing **PR12293-F1** (ADLSHadoopConf consuming raw rather than trimmed option values) or **PR12293-F4** (OAuth token endpoint built from unvalidated `authority_host`/`tenant_id`, allowing plain HTTP / arbitrary hosts). Could you post the rest of the table, or simply reply per finding ID, for the following: 1. **F1** – are the trimmed values now the ones passed to `ADLSHadoopConf`, or is the validator failure mapped to a connector error instead? 2. **F2** – is the HNS (hierarchical namespace) requirement for atomic `tmp_path` → `path` rename now stated in `docs/en/connectors/sink/ADLSFile.md` (and the zh counterpart)? 3. **F3** – are the account_name/container naming rules, auth mutual-exclusion and blocked `hadoop_adls_properties` keys documented, and were the template placeholders updated to pass validation? 4. **F4** – is `authority_host` now restricted to `https` and validated, and is `tenant_id` validated before building the token URL? 5. **F5** – are the en/zh sink option tables now in sync and covering all options exposed by the factories? 6. **F6** – is the `account_key` addition to the default mask list in `ConfigShadeUtils.java` mentioned in the docs and PR description? 7. **F7** – does `docs/en/connectors/source/ADLSFile.md` now show `ONCE` / `EARLIEST` as the defaults for `discovery_mode` / `start_mode`? 8. **F8** – see above. Once I have that mapping I'll do a focused pass on just those spots. Thanks again for the quick turnaround. <!-- streview-comment:1967 --> -- 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]
