Skip to content

Allow script command to load from classpath and file resources - #1386

Closed
ezeprimo wants to merge 2 commits into
spring-projects:mainfrom
ezeprimo:fix/script-classpath-refs-1014
Closed

ezeprimo wants to merge 2 commits into
spring-projects:mainfrom
ezeprimo:fix/script-classpath-refs-1014

Conversation

@ezeprimo

Copy link
Copy Markdown

Fixes #1014

script --file only accepted absolute filesystem paths. It now resolves the value with DefaultResourceLoader, so classpath:... and file:... references work. An existing filesystem path keeps the previous behavior (backward compatible, no new options).

Summary:

  • Script resolves --file/-f as a Spring resource, file-first fallback
  • FileInputProvider gained a Reader overload reusing the line-continuation/comment logic
File Change
spring-shell-core/.../command/Script.java Resource resolution with file-first fallback
spring-shell-core/.../FileInputProvider.java New Reader constructor
spring-shell-core/.../command/ScriptTests.java New: classpath, absolute path, file-URL, missing-file
spring-shell-core/.../FileInputProviderTests.java New Reader test

Test plan:

  • mvn -pl spring-shell-core -am -Dtest=ScriptTests,FileInputProviderTests test: 6/6 green (JDK 17)
  • mvn -pl spring-shell-core -am test: 222/222 green

Resolve the --file value with DefaultResourceLoader so classpath: and file: references work. An existing filesystem path keeps the previous behavior.

Fixes spring-projects#1014

Signed-off-by: ezequiel primon <ezeprimo.ia@gmail.com>
/**
* Tests for {@link Script}.
*
* @author David Pilar

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.

You should use your own name as an author, not mine... ;-)

Signed-off-by: ezequiel primon <ezeprimo.ia@gmail.com>
@ezeprimo

Copy link
Copy Markdown
Author

Author adjusted.

@fmbenhassine

Copy link
Copy Markdown
Contributor

Thank you for the contribution! LGTM, I will merge it. However, I will add a follow-up commit to restrict resource resolution to classpath: and file: prefixes only (as the original OP in #1014 requested). The reasoning is that even though script only ever executes commands already registered in the app, the resource loading step itself (as it is currently implemented in this PR) can be pointed at other URL schemes (like http: or https:), which would make the application issue a network call to a location supplied via the --file option. Restricting to classpath:/file: closes that (security gap) off while resolving #1014 as requested.

fmbenhassine added a commit that referenced this pull request Sep 24, 2026
- Restrict resources to file: and classpath:
- Update Javadocs
- Update documentation

Issue #1014
@fmbenhassine

Copy link
Copy Markdown
Contributor

Rebased, squashed and merged as 07a0e66. Refined in 7fb993b.

Thank you for your contribution!

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.

Extend "script" command to accept classpath references

3 participants