[rubenada]

Jira Link

CALCITE-7860

Changes Proposed

Disable remote fetches by default to comply with threat model. Add new opt-in property to enable this feature.

[zabetak]

Instead of adding two new properties for filtering URLs based on protocols and hosts why not simply going for a single property defining a list of allowed URL prefixes (e.g., calcite.file.urls.allowed). This would simplify documentation, implementation, and usage.

[rubenada]

@zabetak what about proposing something like calcite.file.urls.allowed=https:,http://data.example.com/datasets/ , where:

• An entry that is only a scheme (for example https:) allows any URL with that scheme, whatever its host, port and path.
• Otherwise, in case of a "full" url (for example http://data.example.com/datasets/), it must be a match: the scheme must be equal (case-insensitive), the host must be equal (case-insensitive, exact, so that example.com does not admit my-example.com nor example.com.other.org), the port must be equal (an entry or URL without a port stands for the scheme's default port), if the entry has a path other than /, it must be equal to it or lie beneath it (e.g. http://data.example.com/datasets/t.csv would admitted in our example, but not http://data.example.com/datasets-abc)

[zabetak]

calcite.file.urls.allowed=https:,http://data.example.com/datasets/ is inline with the simpe check I had in mind:

for(String prefix: allowedPrefixes) {
  if(url.startWith(prefix) {
    return true;
  }
}
return false;

As a conf separator the pipe char | is probably safer than comma ,.

[rubenada]

startsWith check might be a bit problematic (https://data.example.com would let in https://data.example.com.bad.com/). I have pushed a refactoring with a bit more thorough validation based on url parts.

[zabetak]

The regex based proposal LGTM! I left some minor refactor/nit suggestions to handle at your discretion.

[rubenada]

Thanks for the review @zabetak ! I think I have applied all your suggestions. If you want to take another look, please let me know if you have any further remark.

[zabetak]

All of my comments are addressed! Many thanks for pushing this @rubenada !

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
82.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[rubenada]

Thanks @zabetak for your help here!