Skip to content

feat(path): add option to normalize forward slash of object name#14

Merged
overtrue merged 1 commit into
rustfs:mainfrom
Littlew0od:fix_issue_569
Jun 7, 2026
Merged

feat(path): add option to normalize forward slash of object name#14
overtrue merged 1 commit into
rustfs:mainfrom
Littlew0od:fix_issue_569

Conversation

@Littlew0od

Copy link
Copy Markdown

PR Description

Summary

According to the s3s-project#569 request, this PR introduces a new feature: normalize_forward_slash, which enables selective normalization of object names. When this feature is disabled, the PR makes no changes to the original behavior.

When the feature is enabled, object names parsed from both path-style requests and virtual-hosted style requests are normalized. The implementation aligns with MinIO's normalization behavior observed during testing, beyond just addressing the issue's requirement of normalizing bucket//object-name.

Observed MinIO behavior:

  • "///" → invalid
  • "//" → invalid
  • "/" → invalid
  • "/object-name" → valid
  • "object-name/" → valid
  • "/object-name//" → valid, treated as "object-name/"
  • "///////////////object-name" → vaild, treated as "object-name"
  • "///////dir////////object-name//" → valid, treated as "dir/object-name/"
  • "dir////////object-name" → invalid
  • "object-name//" → invalid

Based on this, the normalization behavior is defined as follows, strictly aligning with MinIO:

  • Removes all leading slashes (unless the entire path is one or more "/")
  • Replaces consecutive internal slashes with a single slash
  • Preserves a trailing slash if it exists
  • If the object key does not start with a slash, no normalization is performed

Three new tests have been added, covering:

  • Object name normalization for path-style requests
  • Object name normalization for virtual-hosted style requests
  • Unit tests for the normalization function

Related issue

s3s-project#569
rustfs/rustfs#2427

tests

  • just dev
  • cargo nextest run --package s3s --lib --features normalize_forward_slash,minio
  • cargo nextest run --package s3s --lib
  • cargo nextest run --package s3s --lib --features normalize_forward_slash
  • cargo fmt --all --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo check --all-targets

Note

I have submit a same pr in: s3s-project#596, waiting approval.

@Littlew0od Littlew0od force-pushed the fix_issue_569 branch 2 times, most recently from 90e31cd to fba518c Compare June 1, 2026 09:02
@houseme houseme requested review from GatewayJ and overtrue June 7, 2026 01:05
@GatewayJ

GatewayJ commented Jun 7, 2026

Copy link
Copy Markdown
Member

I don’t think this PR should be described as S3-compatible behavior.

For path-style S3 requests, /bucket//keyz should be parsed as:

  • bucket: bucket
  • key: /keyz

The slash after the bucket name is only the bucket/key separator. Everything after that separator is part of the object key. AWS S3 also explicitly documents that S3 URI paths must not be normalized, because normalizing paths changes the object name.

MinIO behavior also does not seem to justify this normalization as a general rule. Historical MinIO/safe-mode behavior may reject object names containing leading or repeated slashes due to filesystem-backed storage constraints. Newer MinIO AIStor
extended object naming handles these cases by encoding special slash patterns on disk and decoding them back to the original object key. In other words, the API-level object key is still preserved; only the internal filesystem representation changes.

This PR does something different: when the parsed key starts with /, it rewrites the key itself. For example:

  • AWS S3: /bucket//keyz -> key /keyz
  • MinIO extended naming: /bucket//keyz -> key /keyz, internally encoded on disk
  • This PR with normalize_forward_slash: /bucket//keyz -> key keyz

That changes the API-visible object name. It is neither AWS S3 semantics nor MinIO extended naming semantics.

If the goal is AWS S3 compatibility, RustFS should accept keys like /keyz and //keyz without rewriting them. If the goal is a legacy MinIO-like workaround, this should be clearly documented as a non-S3-compatible compatibility mode, probably with a
name that reflects that it rewrites object keys rather than normalizes S3 paths.

@GatewayJ

GatewayJ commented Jun 7, 2026

Copy link
Copy Markdown
Member

Reference for Usage Methods

minio server --object-naming=extended /data

It may be inconvenient to modify the compilation parameters here. Users are unable to use the release artifacts.

Signed-off-by: w0od <dingboning02@163.com>
@Littlew0od

Copy link
Copy Markdown
Author

@GatewayJ Thank you for your detailed suggestions. I understand your points and have looked into the relevant documentation. Based on my understanding of the original issue request, its intent was indeed not to establish a path specification for S3-compatible behavior.

Currently, RustFS handles redundant forward slashes by outright rejecting them.
For MinIO, under normal mode, forward slashes are normalized, and redundant slashes are not preserved on disk — meaning key keyz and key /keyz would refer to exactly the same object. In extended object name mode, forward slashes are left untouched from the user's perspective, and are stored on disk using escaped characters.

The changes I plan to make are as follows:

Support two path handling modes: one aligned with MinIO's normal behavior and the other aligned with standard S3 behavior.

  1. Keep the current forward slash normalization behavior, but move it into RustFS's processing logic. (MinIO's normal behavior)
  2. Use escaped characters to allow RustFS to actually store forward slash characters. (Standard S3 behavior, as the default mode)

I plan to move all of this handling logic into RustFS, rather than keeping it in s3s.

Do you think this is a reasonable approach? Thx

@overtrue overtrue merged commit 2f66ffd into rustfs:main Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants