apache / apache/doris

[Bug] PathUtils.equalsIgnoreSchemeIfOneIsS3 path comparison is inconsistent between same-scheme and cross-scheme branches

Open
#64,767 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
15.9k
Forks
3.9k
Avg merge
2d 23h
Merged PRs (30d)
520

Description

### Search before asking

- [x] I had searched in the [issues](https://github.com/apache/doris/issues) and found no similar issues.

### Version

master (`fe-foundation`, `org.apache.doris.foundation.util.PathUtils`).

### What's Wrong?

`PathUtils.equalsIgnoreSchemeIfOneIsS3(p1, p2)` compares two storage-location URIs treating the `s3` scheme as interchangeable with other object-store schemes. Its two branches used **inconsistent** rules:

- **Same scheme** → `p1.equalsIgnoreCase(p2)`: full-string, **case-insensitive**, trailing slash **significant**.
- **Cross-scheme (one is `s3`)** → compares `normalize(authority)`/`normalize(path)` with `Objects.equals`: **case-sensitive**, trailing slash **stripped**.

Consequences:

1. The result for one URI depends on the *other* URI's scheme. For example `s3://bucket/path/` vs `s3://bucket/path` are **unequal** (same-scheme branch), but `s3://bucket/path/` vs `cos://bucket/path` are **equal** (cross-scheme branch).
2. The same-scheme branch ignores case for the whole string, so it can wrongly equate case-sensitive S3 object keys (`s3://b/A` == `s3://b/a`).

The only caller is `HMSTransaction.prepareInsertExistingTable` (`fe-core`), which uses this to decide whether a Hive commit needs a rename — so inconsistent equality can lead to an incorrect rename decision.

### What You Expected?

A single, consistent rule regardless of whether the two schemes match: compare authority + path (scheme ignored when equal or when one side is `s3`), with trailing slashes insignificant and the comparison case-sensitive (object-storage keys are case-sensitive).

### How to Reproduce?

```java
// same pair, different "other" scheme -> different answer (inconsistent):
PathUtils.equalsIgnoreSchemeIfOneIsS3("s3://bucket/path/", "s3://bucket/path"); // false
PathUtils.equalsIgnoreSchemeIfOneIsS3("s3://bucket/path/", "cos://bucket/path"); // true

// same-scheme comparison wrongly ignores case:
PathUtils.equalsIgnoreSchemeIfOneIsS3("s3://bucket/A", "s3://bucket/a"); // true (should be false)
```

### Anything Else?

Fix proposed in the linked PR. It also hardens several edge cases surfaced during review (opaque URIs, percent-encoded slashes, triple-slash / network-path forms) by falling back to exact string comparison for inputs that are malformed for object storage.

### Are you willing to submit PR?

- [x] Yes I am willing to submit a PR!

Contributor guide

Open the contributing guide

Research direction

Start with PathUtils.equalsIgnoreSchemeIfOneIsS3 in org.apache.doris.foundation.util.PathUtils, then inspect its only caller, HMSTransaction.prepareInsertExistingTable. Exercise the reproduction cases from the issue and review the handling of opaque, percent-encoded, triple-slash, and network-path forms. Done means comparison is consistent, case-sensitive, and ignores trailing slashes as specified, with malformed object-storage inputs using exact string comparison.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.