You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "thomasfrederikhoeck (via GitHub)" <gi...@apache.org> on 2024/04/04 20:04:53 UTC
[I] Colon (:) in in object_store::path::{Path} is not handled in Windows [arrow-rs]
thomasfrederikhoeck opened a new issue, #5592:
URL: https://github.com/apache/arrow-rs/issues/5592
**Describe the bug**
When creating a `Path` on Windows colon (`:`) is not sanitized like `<` and `|`.
**To Reproduce**
```rust
use object_store::path::{Path};
fn main() {
let path: String = r"C:\table\time=2021-01-02 03:04:06.000003\file.parquet".to_string();
// let path: Result<Path, object_store::path::Error> = Path::parse(path);
let path_from = Path::from(path);
println!( "{:?}", path_from);
let path: String = r"C:\table\time=2021-01-02 03:04:06.000003\file.parquet".to_string();
let path_parse: Result<Path, object_store::path::Error> = Path::parse(path);
println!( "{:?}", path_parse);
let path: String = r"C:\table\time=2021-01-02 03:04:06.000003\<file|.parquet".to_string();
// let path: Result<Path, object_store::path::Error> = Path::parse(path);
let path_from = Path::from(path);
println!( "{:?}", path_from);
let path: String = r"C:\table\time=2021-01-02 03:04:06.000003\<file|.parquet".to_string();
let path_parse: Result<Path, object_store::path::Error> = Path::parse(path);
println!( "{:?}", path_parse);
}
```
```
Path { raw: "C:%5Ctable%5Ctime=2021-01-02 03:04:06.000003%5Cfile.parquet" }
Ok(Path { raw: "C:\\table\\time=2021-01-02 03:04:06.000003\\file.parquet" })
Path { raw: "C:%5Ctable%5Ctime=2021-01-02 03:04:06.000003%5C%3Cfile%7C.parquet" }
Ok(Path { raw: "C:\\table\\time=2021-01-02 03:04:06.000003\\<file|.parquet" })
```
See SO for forbidden chars https://stackoverflow.com/questions/1976007/what-characters-are-forbidden-in-windows-and-linux-directory-names
**Expected behavior**
I would have expected the output to be
```
Path { raw: "C:%5Ctable%5Ctime=2021-01-02 03%3A04%3A06.000003%5Cfile.parquet" }
Ok(Path { raw: "C:\\table\\time=2021-01-02 03:04:06.000003\\file.parquet" })
Path { raw: "C:%5Ctable%5Ctime=2021-01-02 03%3A04%3A06.000003%5C%3Cfile%7C.parquet" }
Ok(Path { raw: "C:\\table\\time=2021-01-02 03:04:06.000003\\<file|.parquet" })
```
**Additional context**
This came up here https://github.com/delta-io/delta-rs/issues/2382
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Colon (:) in in object_store::path::{Path} is not handled on Windows [arrow-rs]
Posted by "thomasfrederikhoeck (via GitHub)" <gi...@apache.org>.
thomasfrederikhoeck commented on issue #5592:
URL: https://github.com/apache/arrow-rs/issues/5592#issuecomment-2043217548
@tustvold if you think that solution is good I can give it a go.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Colon (:) in in object_store::path::{Path} is not handled on Windows [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #5592: Colon (:) in in object_store::path::{Path} is not handled on Windows
URL: https://github.com/apache/arrow-rs/issues/5592
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Colon (:) in in object_store::path::{Path} is not handled on Windows [arrow-rs]
Posted by "hesampakdaman (via GitHub)" <gi...@apache.org>.
hesampakdaman commented on issue #5592:
URL: https://github.com/apache/arrow-rs/issues/5592#issuecomment-2143398812
Thanks @tustvold! I'll gladly help out but might need some guidance to drive it home. I've created a draft, appreciate if you could take a look to see if I'm on the right track :)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Colon (:) in in object_store::path::{Path} is not handled on Windows [arrow-rs]
Posted by "thomasfrederikhoeck (via GitHub)" <gi...@apache.org>.
thomasfrederikhoeck commented on issue #5592:
URL: https://github.com/apache/arrow-rs/issues/5592#issuecomment-2049541066
@tustvold I did a lot of googling and from what I can tell there is no default way. I think it is a good route to go to just explicitly escape them. Maybe just starting with `:` since I expect it to be much more heavily used for data given it is used for timestamps compared to the other chars.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Colon (:) in in object_store::path::{Path} is not handled on Windows [arrow-rs]
Posted by "thomasfrederikhoeck (via GitHub)" <gi...@apache.org>.
thomasfrederikhoeck commented on issue #5592:
URL: https://github.com/apache/arrow-rs/issues/5592#issuecomment-2143466663
I agree with @tustvold that it should windows-only but I think it should be true be default given that it can never be a valid path on windows.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Colon (:) in in object_store::path::{Path} is not handled on Windows [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #5592:
URL: https://github.com/apache/arrow-rs/issues/5592#issuecomment-2043432595
I think probably we should add specific handling for LocalFilesystem on Windows. Just because Windows filesystems are a complete mess, I think that shouldn't leak out.
Perhaps we could explicitly escape them or something on Windows platforms? Is there standard way the windows ecosystem works around this limitation?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Colon (:) in in object_store::path::{Path} is not handled on Windows [arrow-rs]
Posted by "thomasfrederikhoeck (via GitHub)" <gi...@apache.org>.
thomasfrederikhoeck commented on issue #5592:
URL: https://github.com/apache/arrow-rs/issues/5592#issuecomment-2039722699
@tustvold I guess it would be breaking but would anyone rely on a path that can't exists? Or does this sanitizing happen for the same set of chars across all platforms?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Colon (:) in in object_store::path::{Path} is not handled on Windows [arrow-rs]
Posted by "hesampakdaman (via GitHub)" <gi...@apache.org>.
hesampakdaman commented on issue #5592:
URL: https://github.com/apache/arrow-rs/issues/5592#issuecomment-2142943951
> It is the same set of chars across all platforms, although I suppose we could just add some special logic for windows within LocalFilesystem...
I was going to suggest to have several platform specific `INVALID` consts :joy:
```diff
modified object_store/src/path/parts.rs
@@ -73,6 +73,7 @@ impl<'a> PathPart<'a> {
}
/// Characters we want to encode.
+#[cfg(target_family="windows")]
const INVALID: &AsciiSet = &CONTROLS
// The delimiter we are reserving for internal hierarchy
.add(DELIMITER_BYTE)
@@ -97,8 +98,38 @@ const INVALID: &AsciiSet = &CONTROLS
.add(b'\r')
.add(b'\n')
.add(b'*')
+ .add(b':')
.add(b'?');
```
> I think probably we should add specific handling within LocalFilesystem on Windows. Just because Windows filesystems are a complete mess, I don't think that shouldn't leak out. This would also avoid a breaking change
I didn't have a thorough look but does it suffice to have this special logic in `path_to_filesystem`?
```rust
/// Return an absolute filesystem path of the given file location
pub fn path_to_filesystem(&self, location: &Path) -> Result<PathBuf> {
ensure!(
is_valid_file_path(location),
InvalidPathSnafu {
path: location.as_ref()
}
);
self.config.prefix_to_filesystem(location)
}
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Colon (:) in in object_store::path::{Path} is not handled on Windows [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #5592:
URL: https://github.com/apache/arrow-rs/issues/5592#issuecomment-2143042237
It would be a breaking change, not to mention extremely disruptive, to add `:` to the Invalid set, as this set applies to all paths regardless of store. Given Windows LocalFilesystem is the only store with this problem, and colons are very common, I think the least disruptive change would be to add some fallback logic in LocalFilesystem on Windows that escapes them, potentially disabled by default.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Colon (:) in in object_store::path::{Path} is not handled on Windows [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #5592:
URL: https://github.com/apache/arrow-rs/issues/5592#issuecomment-2039228381
This would be a breaking change but probably makes sense on balance
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Colon (:) in in object_store::path::{Path} is not handled on Windows [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #5592:
URL: https://github.com/apache/arrow-rs/issues/5592#issuecomment-2039735629
It is the same set of chars across all platforms, although I suppose we could just add some special logic for windows within LocalFilesystem...
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org