Skip to content

Commit b46e3ff

Browse files
bentshermanbentsherman
committed
cleanup
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
1 parent c6939e3 commit b46e3ff

10 files changed

Lines changed: 79 additions & 66 deletions

File tree

modules/nextflow/src/main/groovy/nextflow/exception/IllegalModulePath.groovy renamed to modules/nextflow/src/main/groovy/nextflow/exception/IllegalModulePathException.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,5 @@ import groovy.transform.InheritConstructors
2424
* @author Paolo Di Tommaso <paolo.ditommaso@gmail.com>
2525
*/
2626
@InheritConstructors
27-
class IllegalModulePath extends Exception {
27+
class IllegalModulePathException extends Exception {
2828
}

‎modules/nextflow/src/main/groovy/nextflow/module/DefaultRemoteModuleResolver.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import nextflow.Global
2222
import nextflow.config.ConfigBuilder
2323

2424
import nextflow.config.RegistryConfig
25-
import nextflow.exception.IllegalModulePath
25+
import nextflow.exception.IllegalModulePathException
2626
import nextflow.module.spi.RemoteModuleResolver
2727

2828
import java.nio.file.Path
@@ -65,7 +65,7 @@ class DefaultRemoteModuleResolver implements RemoteModuleResolver {
6565
log.debug "Module ${reference} resolved to ${mainFile}"
6666
return mainFile
6767
} catch (Exception e) {
68-
throw new IllegalModulePath("Failed to resolve remote module ${moduleName}: ${e.message}", e)
68+
throw new IllegalModulePathException("Failed to resolve remote module ${moduleName}: ${e.message}", e)
6969
}
7070
}
7171

‎modules/nextflow/src/main/groovy/nextflow/script/IncludeDef.groovy‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import groovy.transform.PackageScope
2727
import groovy.util.logging.Slf4j
2828
import nextflow.NF
2929
import nextflow.Session
30-
import nextflow.exception.IllegalModulePath
30+
import nextflow.exception.IllegalModulePathException
3131
import nextflow.exception.ScriptCompilationException
3232
import nextflow.module.ModuleReference
3333
import nextflow.module.spi.RemoteModuleResolverProvider
@@ -167,7 +167,7 @@ class IncludeDef {
167167
final result = include as Path
168168
if( result.isAbsolute() ) {
169169
if( result.scheme == 'file' ) return result
170-
throw new IllegalModulePath("Cannot resolve module path: ${result.toUriString()}")
170+
throw new IllegalModulePathException("Cannot resolve module path: ${result.toUriString()}")
171171
}
172172
final str = include.toString()
173173
if( str.startsWith('./') || str.startsWith('../') ) {
@@ -212,10 +212,10 @@ class IncludeDef {
212212
@PackageScope
213213
void checkValidPath(path) {
214214
if( !path )
215-
throw new IllegalModulePath("Missing module path attribute")
215+
throw new IllegalModulePathException("Missing module path attribute")
216216

217217
if( path instanceof Path && path.scheme != 'file' )
218-
throw new IllegalModulePath("Remote modules are not allowed -- Offending module: ${path.toUriString()}")
218+
throw new IllegalModulePathException("Remote modules are not allowed -- Offending module: ${path.toUriString()}")
219219

220220
final str = path.toString()
221221
if( str.startsWith('/') || str.startsWith('./') || str.startsWith('../') || str.startsWith('plugin/') )
@@ -225,7 +225,7 @@ class IncludeDef {
225225
try {
226226
ModuleReference.parse(str)
227227
} catch( Exception e ) {
228-
throw new IllegalModulePath("Module path must start with '/', './', '../' or 'plugin/' prefix, or be a valid remote module reference (scope/name) -- Offending module: $str")
228+
throw new IllegalModulePathException("Module path must start with '/', './', '../' or 'plugin/' prefix, or be a valid remote module reference (scope/name) -- Offending module: $str")
229229
}
230230
}
231231

‎modules/nextflow/src/main/groovy/nextflow/script/parser/v2/ScriptCompiler.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import groovy.lang.GroovyClassLoader;
3333
import groovy.lang.GroovyCodeSource;
3434
import com.google.common.hash.Hashing;
35-
import nextflow.Global;
3635
import nextflow.script.ast.RecordNode;
3736
import nextflow.script.ast.WorkflowNode;
3837
import nextflow.script.control.CallSiteCollector;
@@ -89,16 +88,18 @@ public class ScriptCompiler {
8988

9089
private final CompilerConfiguration config;
9190
private final GroovyClassLoader loader;
91+
private final Path projectDir;
9292

9393
private Compiler compiler;
9494

95-
public ScriptCompiler(boolean debug, Path targetDirectory, ClassLoader parent) {
96-
this(getConfig(debug, targetDirectory), parent);
95+
public ScriptCompiler(boolean debug, Path targetDirectory, ClassLoader parent, Path projectDir) {
96+
this(getConfig(debug, targetDirectory), parent, projectDir);
9797
}
9898

99-
public ScriptCompiler(CompilerConfiguration config, ClassLoader parent) {
99+
public ScriptCompiler(CompilerConfiguration config, ClassLoader parent, Path projectDir) {
100100
this.config = config;
101101
this.loader = new GroovyClassLoader(parent, config);
102+
this.projectDir = projectDir;
102103
}
103104

104105
private static CompilerConfiguration getConfig(boolean debug, Path targetDirectory) {
@@ -269,9 +270,8 @@ public void addPhaseOperation(IPrimaryClassNodeOperation op, int phase) {
269270

270271
private void analyze(SourceUnit source) {
271272
// on first pass, recursively add included modules to queue
272-
Path baseDir = Global.getSession() != null ? Global.getSession().getBaseDir() : null;
273273
if( entry == null ) {
274-
modules = new ModuleResolver(compiler, baseDir).resolve(source, uri -> createSourceUnit(uri));
274+
modules = new ModuleResolver(projectDir, compiler).resolve(source, uri -> createSourceUnit(uri));
275275
for( var su : modules )
276276
addSource(su);
277277
entry = source;
@@ -285,7 +285,7 @@ private void analyze(SourceUnit source) {
285285
var cn = source.getAST().getClasses().get(0);
286286

287287
// perform strict syntax checking
288-
var includeResolver = new ResolveIncludeVisitor(source, compiler, baseDir);
288+
var includeResolver = new ResolveIncludeVisitor(source, projectDir, compiler);
289289
includeResolver.visit();
290290
for( var error : includeResolver.getErrors() )
291291
source.getErrorCollector().addErrorAndContinue(error);

‎modules/nextflow/src/main/groovy/nextflow/script/parser/v2/ScriptLoaderV2.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ class ScriptLoaderV2 implements ScriptLoader {
175175

176176
private ScriptCompiler getCompiler() {
177177
if( !compiler )
178-
compiler = new ScriptCompiler(session.debug, session.classesDir, session.getClassLoader())
178+
compiler = new ScriptCompiler(session.debug, session.classesDir, session.getClassLoader(), session.baseDir)
179179
return compiler
180180
}
181181

‎modules/nextflow/src/test/groovy/nextflow/script/IncludeDefTest.groovy‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import java.nio.file.NoSuchFileException
2424
import java.nio.file.Path
2525

2626
import nextflow.ast.NextflowDSL
27-
import nextflow.exception.IllegalModulePath
27+
import nextflow.exception.IllegalModulePathException
2828
import nextflow.file.FileHelper
2929
import org.codehaus.groovy.control.CompilerConfiguration
3030
import org.codehaus.groovy.control.customizers.ASTTransformationCustomizer
@@ -51,7 +51,7 @@ class IncludeDefTest extends Specification {
5151
when:
5252
include.resolveModulePath('http://foo.com/bar')
5353
then:
54-
thrown(IllegalModulePath)
54+
thrown(IllegalModulePathException)
5555

5656
}
5757

@@ -147,17 +147,17 @@ class IncludeDefTest extends Specification {
147147
when:
148148
include.checkValidPath('invalid!')
149149
then:
150-
thrown(IllegalModulePath)
150+
thrown(IllegalModulePathException)
151151

152152
when:
153153
include.checkValidPath( 'http://foo.com/x.y')
154154
then:
155-
thrown(IllegalModulePath)
155+
thrown(IllegalModulePathException)
156156

157157
when:
158158
include.checkValidPath(FileHelper.asPath('http://foo.com/x/y/z'))
159159
then:
160-
thrown(IllegalModulePath)
160+
thrown(IllegalModulePathException)
161161

162162
}
163163

‎modules/nf-lang/src/main/java/nextflow/module/spi/FallbackRemoteModuleResolver.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,14 @@ public class FallbackRemoteModuleResolver implements RemoteModuleResolver {
3232

3333
@Override
3434
public Path resolve(String moduleName, Path projectDir) {
35-
final Path modulesDir = (projectDir == null) ? projectDir.resolve("modules") : Path.of("modules").toAbsolutePath();
36-
final var resolved = modulesDir.resolve(moduleName).normalize();
37-
// Prevent path traversal outside the base directory
38-
if (!resolved.startsWith(modulesDir.normalize())) {
39-
throw new IllegalStateException("Invalid module name '" + moduleName + "' - path escapes the modules directory");
35+
var baseDir = projectDir != null ? projectDir : Path.of(".").toAbsolutePath();
36+
var modulesDir = baseDir.resolve("modules").normalize();
37+
var resolved = modulesDir.resolve(moduleName).normalize();
38+
if( !resolved.startsWith(modulesDir) ) {
39+
throw new IllegalStateException("Invalid module name '" + moduleName + "' -- path escapes the modules directory");
4040
}
41-
if (!Files.exists(resolved)) {
42-
throw new IllegalStateException("Module '" + moduleName + "' not locally found at 'modules' folder - use 'nextflow module install' to download module files");
41+
if( !Files.exists(resolved) ) {
42+
throw new IllegalStateException("Module '" + moduleName + "' not found in 'modules' directory -- use 'nextflow module install' to install module first");
4343
}
4444
return resolved.resolve("main.nf");
4545
}

‎modules/nf-lang/src/main/java/nextflow/script/control/ModuleResolver.java‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,11 @@ public class ModuleResolver {
3939
private Compiler compiler;
4040
private Path projectDir;
4141

42-
public ModuleResolver(Compiler compiler, Path projectDir) {
42+
public ModuleResolver(Path projectDir, Compiler compiler) {
4343
this.compiler = compiler;
4444
this.projectDir = projectDir;
4545
}
4646

47-
public ModuleResolver(Compiler compiler) {
48-
this(compiler, null);
49-
}
50-
5147
/**
5248
* Resolve all modules included by a script.
5349
*
@@ -80,9 +76,8 @@ private SourceUnit resolveInclude(IncludeNode node, SourceUnit sourceUnit, Funct
8076
if( source.startsWith("plugin/") )
8177
return null;
8278

83-
var includeUri = isRemoteModule(source) ?
84-
RemoteModuleResolverProvider.getInstance().resolve(source, projectDir).normalize().toUri() :
85-
getIncludeUri(Path.of(sourceUnit.getSource().getURI()).getParent(), source);
79+
var uri = sourceUnit.getSource().getURI();
80+
var includeUri = getIncludeUri(uri, source);
8681
if( compiler.getSource(includeUri) != null )
8782
return null;
8883
if( !Files.exists(Path.of(includeUri)) )
@@ -95,20 +90,33 @@ private SourceUnit resolveInclude(IncludeNode node, SourceUnit sourceUnit, Funct
9590
return includeSource;
9691
}
9792

93+
private URI getIncludeUri(URI uri, String source) {
94+
if( isRemoteModule(source) ) {
95+
return RemoteModuleResolverProvider.getInstance()
96+
.resolve(source, projectDir)
97+
.normalize()
98+
.toUri();
99+
}
100+
else {
101+
var parent = Path.of(uri).getParent();
102+
return getLocalIncludeUri(parent, source);
103+
}
104+
}
105+
98106
/**
99107
* Module name pattern matching the canonical format used by ModuleReference.
100108
* Scope: lowercase alphanumeric with dots/underscores/hyphens.
101109
* Name: one or more slash-separated segments, each lowercase alphanumeric with dots/underscores/hyphens.
102110
*/
103-
static final String REMOTE_MODULE_PATTERN = "^[a-z0-9][a-z0-9._\\-]*/[a-z][a-z0-9._\\-]*(/[a-z][a-z0-9._\\-]*)*$";
111+
private static final String REMOTE_MODULE_PATTERN = "^[a-z0-9][a-z0-9._\\-]*/[a-z][a-z0-9._\\-]*(/[a-z][a-z0-9._\\-]*)*$";
104112

105113
static boolean isRemoteModule(String source) {
106114
if( source.startsWith("/") || source.startsWith("./") || source.startsWith("../") )
107115
return false;
108116
return source.matches(REMOTE_MODULE_PATTERN);
109117
}
110118

111-
private static URI getIncludeUri(Path parent, String source) {
119+
private static URI getLocalIncludeUri(Path parent, String source) {
112120
Path includePath = parent.resolve(source);
113121
if( Files.isDirectory(includePath) )
114122
includePath = includePath.resolve("main.nf");

‎modules/nf-lang/src/main/java/nextflow/script/control/ResolveIncludeVisitor.java‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ public class ResolveIncludeVisitor extends ScriptVisitorSupport {
4949

5050
private URI uri;
5151

52+
private Path projectDir;
53+
5254
private Compiler compiler;
5355

5456
private Set<URI> changedUris;
@@ -57,22 +59,16 @@ public class ResolveIncludeVisitor extends ScriptVisitorSupport {
5759

5860
private boolean changed;
5961

60-
private Path projectDir;
61-
62-
public ResolveIncludeVisitor(SourceUnit sourceUnit, Compiler compiler, Set<URI> changedUris, Path projectDir) {
62+
public ResolveIncludeVisitor(SourceUnit sourceUnit, Path projectDir, Compiler compiler, Set<URI> changedUris) {
6363
this.sourceUnit = sourceUnit;
6464
this.uri = sourceUnit.getSource().getURI();
6565
this.compiler = compiler;
6666
this.changedUris = changedUris;
6767
this.projectDir = projectDir;
6868
}
6969

70-
public ResolveIncludeVisitor(SourceUnit sourceUnit, Compiler compiler, Path projectDir) {
71-
this(sourceUnit, compiler, null, projectDir);
72-
}
73-
74-
public ResolveIncludeVisitor(SourceUnit sourceUnit, Compiler compiler) {
75-
this(sourceUnit, compiler, null, null);
70+
public ResolveIncludeVisitor(SourceUnit sourceUnit, Path projectDir, Compiler compiler) {
71+
this(sourceUnit, projectDir, compiler, null);
7672
}
7773

7874
@Override
@@ -95,18 +91,14 @@ public void visitInclude(IncludeNode node) {
9591
}
9692

9793
URI includeUri;
98-
if( ModuleResolver.isRemoteModule(source) ) {
99-
try {
100-
includeUri = RemoteModuleResolverProvider.getInstance().resolve(source, projectDir).normalize().toUri();
101-
}
102-
catch( IllegalStateException e ) {
103-
addError(e.getMessage(), node);
104-
return;
105-
}
94+
try {
95+
includeUri = getIncludeUri(source);
10696
}
107-
else {
108-
includeUri = getIncludeUri(Path.of(sourceUnit.getSource().getURI()).getParent(), source);
97+
catch( Exception e ) {
98+
addError(e.getMessage(), node);
99+
return;
109100
}
101+
110102
if( !isIncludeStale(node, includeUri) )
111103
return;
112104
changed = true;
@@ -144,7 +136,20 @@ private static void setPlaceholderTargets(IncludeNode node) {
144136
}
145137
}
146138

147-
private static URI getIncludeUri(Path parent, String source) {
139+
private URI getIncludeUri(String source) {
140+
if( ModuleResolver.isRemoteModule(source) ) {
141+
return RemoteModuleResolverProvider.getInstance()
142+
.resolve(source, projectDir)
143+
.normalize()
144+
.toUri();
145+
}
146+
else {
147+
var parent = Path.of(uri).getParent();
148+
return getLocalIncludeUri(parent, source);
149+
}
150+
}
151+
152+
private static URI getLocalIncludeUri(Path parent, String source) {
148153
Path includePath = parent.resolve(source);
149154
if( Files.isDirectory(includePath) )
150155
includePath = includePath.resolve("main.nf");

‎modules/nf-lang/src/main/java/nextflow/script/control/ScriptParser.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,18 +34,18 @@
3434
*/
3535
public class ScriptParser {
3636

37-
private Compiler compiler;
3837
private Path projectDir;
39-
40-
public ScriptParser() {
41-
this(null);
42-
}
38+
private Compiler compiler;
4339

4440
public ScriptParser(Path projectDir) {
41+
this.projectDir = projectDir;
4542
var config = getConfig();
4643
var classLoader = new GroovyClassLoader();
4744
this.compiler = new Compiler(config, classLoader);
48-
this.projectDir = projectDir;
45+
}
46+
47+
public ScriptParser() {
48+
this(null);
4949
}
5050

5151
public Compiler compiler() {
@@ -75,11 +75,11 @@ private void parse0(SourceUnit source) {
7575
public void analyze() {
7676
var sources = new ArrayList<>(compiler.getSources().values());
7777
for( var source : sources ) {
78-
new ModuleResolver(compiler(), projectDir).resolve(source, (uri) -> compiler.createSourceUnit(new File(uri)));
78+
new ModuleResolver(projectDir, compiler()).resolve(source, (uri) -> compiler.createSourceUnit(new File(uri)));
7979
}
8080

8181
for( var source : compiler.getSources().values() ) {
82-
var includeResolver = new ResolveIncludeVisitor(source, compiler, projectDir);
82+
var includeResolver = new ResolveIncludeVisitor(source, projectDir, compiler);
8383
includeResolver.visit();
8484
for( var error : includeResolver.getErrors() )
8585
source.getErrorCollector().addErrorAndContinue(error);

0 commit comments

Comments
 (0)
⚡