Skip to content

maven plugin now concats data and source params streams. - #190

Merged
phax merged 4 commits into
phax:pr190from
glelouet:plugin_allow_data_and_source
Sep 7, 2026
Merged

maven plugin now concats data and source params streams.#190
phax merged 4 commits into
phax:pr190from
glelouet:plugin_allow_data_and_source

Conversation

@glelouet

@glelouet glelouet commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

STATUS : stale
A potential issue with using stream of URL arose, a security check needs to be performed


Previously, in the plugin params, if "data" was set then "source" was ignored ; now they are both used, providing several inputstreams to apply the generator against.

If none are present, the plugin still resolves a Stream.of(null) : this way a generator which does not need data would still work (is called against a null inputstream), while a generator that requires data would fail (throwing NPE) rather than be silently skipped.

Also fixes the plugin not trying to fetch the source as an url, and showing errors when no error happened.

Also fixes the plugin silently skipping when the source could not be loaded.

This was possible after the patch listing the children when the source is a directory, as this patch enabled having several sources.

Previously, "data" was superseeding the "source" ; now they are both
used.

If none are present, still returns a Stream.of(null) so that a generator
which does not need data would still work (is called against a null
inputstream), while a generator that requires data would fail (with NPE)
rather than be silently skipped.
@glelouet
glelouet requested a review from phax September 6, 2026 09:33
@phax

phax commented Sep 6, 2026

Copy link
Copy Markdown
Owner

This piece of code should afterwards be assessed from a security perspective - besides that it looks good for me

* plugin sources are kept with the inputstream

Now the source responsible is returned in the error

* sourcedinputstream transmitted to the generators.

* activate specific projects formatter and reformat them :P

---------

Co-authored-by: glelouet <n@mo>
@glelouet

glelouet commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator Author

This piece of code should afterwards be assessed from a security perspective - besides that it looks good for me

waht ? I don't get it.

@glelouet

glelouet commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator Author

@phax I made a few changes to merge the other PR correctly. Please verify again.

since the plugin should never had no data to transmit, no point in this
method that was not called to start with.
@phax

phax commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Because we work with streams of URLs we need to see if this can be used to get in remote data we don't want to

@glelouet

glelouet commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator Author

Because we work with streams of URLs we need to see if this can be used to get in remote data we don't want to

No ?

We work with SourcedInputStream, one of which at most is created from an url.

<data>my raw data, to anger Gordon Ramsay</data>
<source>http://my.source.txt</source>

can only result in one inputstream based on the raw data, and one inputstream based on the url.

Maybe it's still an issue, you're right. But the count of URL-based stream is (0,1)

@glelouet

glelouet commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

@phax should I return a list of SourcedInputStream instead ? In the present calls, the stream is used with .tolist() so that would barely change anything, while not having a stream containing indirect url(s) .

@phax

phax commented Sep 7, 2026

Copy link
Copy Markdown
Owner

No idea - I would need to look at it once it is merged - but I currently have a different focus

@glelouet

glelouet commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

Okay.

Since it's a potential security issue, I rather draft it, and wait until you find the time to be sure there is no added issue.

@glelouet
glelouet marked this pull request as draft September 7, 2026 13:33
@glelouet
glelouet requested a review from phax September 7, 2026 13:34
@phax

phax commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Yes please draft it - I will merge in a temp branch and check what I can do with it ;-)

@glelouet

glelouet commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

Already did.

I also re requested a review. Does not mean you have to do it now :)

I update the first post. I already updated it to present the improvement in a way that can be easily integrated (I believe) during the version release.

@glelouet glelouet added bug enhancement patch patch-level change : no addition of feature, no breaking change. stale minor minor-level change : new feature, no breaking change and removed patch patch-level change : no addition of feature, no breaking change. labels Sep 7, 2026
@phax
phax changed the base branch from master to pr190 September 7, 2026 13:52
@phax

phax commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Please mark the PR as ready

@phax
phax marked this pull request as ready for review September 7, 2026 14:10
@phax
phax merged commit 3c815ee into phax:pr190 Sep 7, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug enhancement minor minor-level change : new feature, no breaking change stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants