Skip to content

Document illegal windows filesystem characters - #12974

Merged
elharo merged 3 commits into
masterfrom
win
Aug 31, 2026
Merged

Document illegal windows filesystem characters#12974
elharo merged 3 commits into
masterfrom
win

Conversation

@elharo

@elharo elharo commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

I feel there's a missing layer of indirection somewhere. Repository IDs and version strings should not be tightly coupled to the vagaries of one filesystem. We might want to untangle that. But for the moment this change at least makes the nature of the concern more explicit.

@elharo
elharo requested a review from gnodet August 31, 2026 11:47
@elharo elharo added the documentation Improvements or additions to documentation label Aug 31, 2026

@elharo elharo left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this is not a complete fix for Windows. Windows reserves a number of file names at the word -- not character level. If any of these showed up as a group ID or artifact ID or something else we try to create a file for in the local repo, Windows developers would have a bad day:

CON (Console)
PRN (Printer)
AUX (Auxiliary device)
NUL (Null device)
COM1 through COM9 (Serial communication ports)
LPT1 through LPT9 (Parallel printer ports)

@gnodet

gnodet commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

I feel there's a missing layer of indirection somewhere. Repository IDs and version strings should not be tightly coupled to the vagaries of one filesystem. We might want to untangle that. But for the moment this change at least makes the nature of the concern more explicit.

Those constants are used to validate the <parent><relativePath> model element. It's an actual relative path on the file system.

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean readability improvement — the renamed constants (ILLEGAL_NTFS_FILENAME_CHARS, ILLEGAL_WINDOWS_PATH_CHARS) and their Javadoc make the Windows filesystem constraint explicit rather than implicit.

One observation: DefaultSettingsValidator.java has a duplicated ILLEGAL_REPO_ID_CHARS constant with a comment // ILLEGAL_FS_CHARS that still references the old name. The compat modules (compat/maven-model-builder/.../DefaultModelValidator.java and compat/maven-settings-builder/.../DefaultSettingsValidator.java) also have their own copies of these constants with the old names. Consider extending the rename to those files for consistency — though the compat modules are lower priority since they mirror the main implementation.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

@gnodet

gnodet commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Also this is not a complete fix for Windows. Windows reserves a number of file names at the word -- not character level. If any of these showed up as a group ID or artifact ID or something else we try to create a file for in the local repo, Windows developers would have a bad day:

CON (Console)
PRN (Printer)
AUX (Auxiliary device)
NUL (Null device)
COM1 through COM9 (Serial communication ports)
LPT1 through LPT9 (Parallel printer ports)

I don't think this is an issue. It's only used to read the pom file relative to an existing pom.xml. And the goal of the previous PR was to reject known invalid paths before they cause an unwanted exception while constructing new Path(xxx). We don't necessarily want to verify that the path points to a valid directory or POM file, this is done later when loading the model. We just want to make sure that it looks like a valid path.

@elharo
elharo merged commit 24d371b into master Aug 31, 2026
23 checks passed
@elharo
elharo deleted the win branch August 31, 2026 15:52
@github-actions github-actions Bot added this to the 4.1.0 milestone Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants