Nayana-Naik73 opened a new pull request, #55250:
URL: https://github.com/apache/arrow/pull/55250
### Rationale for this change
`DirectoryPartitioning` and `FilenamePartitioning` build the write path from
`Scalar::ToString()` of each partition value with no escaping
(`KeyValuePartitioning::FormatPartitionSegments`), unlike `HivePartitioning`
which URI-escapes values and prefixes them with `key=`. The partition values
come from the data being written, so a value of `..` or one containing `/`
turns into extra path components and the dataset writer creates files outside
`base_dir`. With pyarrow 21.0.0:
```python
t = pa.table({"x": [1, 2], "part": ["../../escaped", "a"]})
ds.write_dataset(t, "/tmp/x/out", format="parquet",
partitioning=ds.partitioning(pa.schema([("part",
pa.string())])))
# writes /tmp/escaped/part-0.parquet
```
The filename flavor does the same through the file name prefix
(`../../fn_part-0.parquet`). With `existing_data_behavior="delete_matching"`, a
value of `..` makes `DatasetWriter` call `DeleteDirContents` on `base_dir/..`,
which deleted `base_dir` itself in my test before it errored out.
### What changes are included in this PR?
`FormatPartitionSegments` now returns `Status::Invalid` when a formatted
value is `.` or `..` or contains `/`. Valid values format exactly as before.
Writing such values never round-tripped anyway (GH-33507), so rejecting them is
the minimal fix here; URI-escaping these flavors like Hive does would change
on-disk names for many more values and seems better as a separate discussion.
### Are these changes tested?
Yes. Added `TestPartitioning.DirectoryPartitioningFormatInvalidSegment`,
which fails without the fix and passes with it. The rest of the
`arrow-dataset-*` tests pass locally.
### Are there any user-facing changes?
Writing a dataset with directory or filename partitioning now fails when a
partition value is `.`, `..` or contains `/`, instead of writing to an
unintended location.
**This PR contains a "Critical Fix".** Partition values taken from the
written data could make the dataset writer create, and with `delete_matching`
delete, files outside `base_dir`.
### Was AI used for this PR?
In accordance to the [AI generation
guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code),
please disclose below whether and how AI was used in this PR.
**PR code and description written by:**
- [X] Human
- [x] AI
**Reviewed before submission by:**
- [x] Human
- [ ] AI
- [ ] Not reviewed
--
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]
