Skip to content

v1_parser: use URI#open instead of URI.open #4854

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 6, 2025
Merged

v1_parser: use URI#open instead of URI.open #4854

merged 1 commit into from
Mar 6, 2025

Conversation

Watson1978
Copy link
Contributor

@Watson1978 Watson1978 commented Mar 6, 2025 •
edited by daipom
Loading

Which issue(s) this PR fixes:
Fixes #

What this PR does / why we need it:
By CodeQL documentation, it is safer to avoid using URI.open.
This is a false positive, so there is no impact on Fluentd.
(Because URI.parse checks the uri in advance.)

This is similar with #4848

Docs Changes:

Release Note:

@Watson1978 Watson1978 added the backport to LTS We will backport this fix to the LTS branch label Mar 6, 2025
@Watson1978 Watson1978 added this to the v1.19.0 milestone Mar 6, 2025
@Watson1978 Watson1978 marked this pull request as ready for review March 6, 2025 05:25
@Watson1978 Watson1978 requested a review from daipom March 6, 2025 05:25
@Watson1978 Watson1978 force-pushed the codeql/v1_parser branch 3 times, most recently from 04a12fb to 0254d7e Compare March 6, 2025 07:57
daipom
daipom reviewed Mar 6, 2025
View reviewed changes
Copy link
Contributor

@daipom daipom left a comment

Choose a reason for hiding this comment

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

Thanks.

It appears to me that v1parser can recognize Windows absolute paths (C:/foo).
Can't we just use u.open?

@Watson1978
Copy link
Contributor Author

Thanks.

        if u.scheme == 'file' || (!u.scheme.nil? && u.scheme.length == 1) || u.path == uri.tr(' ', '+') # file path
          # When the Windows absolute path then u.scheme.length == 1

Indeed, seems it recognize Windows path.
I will fix the patch

@Watson1978
v1_parser: use URI#open instead of URI.open
d6aa891 
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
daipom
daipom approved these changes Mar 6, 2025
View reviewed changes
Copy link
Contributor

@daipom daipom left a comment

Choose a reason for hiding this comment

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

Thanks!

@daipom daipom merged commit ccf5768 into master Mar 6, 2025
13 checks passed
@daipom daipom deleted the codeql/v1_parser branch March 6, 2025 09:56
kenhys pushed a commit to kenhys/fluentd that referenced this pull request Apr 23, 2025
@Watson1978 @kenhys
v1_parser: use URI#open instead of URI.open (fluent#4854)
4f7cf08 
**Which issue(s) this PR fixes**:
Fixes #

**What this PR does / why we need it**:
By [CodeQL
documentation](https://codeql.github.com/codeql-query-help/ruby/rb-non-constant-kernel-open/),
it is safer to avoid using `URI.open`.

This is similar with fluent#4848

**Docs Changes**:

**Release Note**:

Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
kenhys pushed a commit to kenhys/fluentd that referenced this pull request Apr 23, 2025
@Watson1978 @kenhys
v1_parser: use URI#open instead of URI.open (fluent#4854)
29e5cd7 
**Which issue(s) this PR fixes**:
Fixes #

**What this PR does / why we need it**:
By [CodeQL
documentation](https://codeql.github.com/codeql-query-help/ruby/rb-non-constant-kernel-open/),
it is safer to avoid using `URI.open`.

This is similar with fluent#4848

**Docs Changes**:

**Release Note**:

Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
kenhys pushed a commit to kenhys/fluentd that referenced this pull request Apr 23, 2025
@Watson1978 @kenhys
v1_parser: use URI#open instead of URI.open (fluent#4854)
5a3d6ef 
**Which issue(s) this PR fixes**:
Fixes #

**What this PR does / why we need it**:
By [CodeQL
documentation](https://codeql.github.com/codeql-query-help/ruby/rb-non-constant-kernel-open/),
it is safer to avoid using `URI.open`.

This is similar with fluent#4848

**Docs Changes**:

**Release Note**:

Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
kenhys pushed a commit to kenhys/fluentd that referenced this pull request Apr 23, 2025
@Watson1978 @kenhys
v1_parser: use URI#open instead of URI.open (fluent#4854)
13d334c 
**Which issue(s) this PR fixes**:
Fixes #

**What this PR does / why we need it**:
By [CodeQL
documentation](https://codeql.github.com/codeql-query-help/ruby/rb-non-constant-kernel-open/),
it is safer to avoid using `URI.open`.

This is similar with fluent#4848

**Docs Changes**:

**Release Note**:

Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
@kenhys kenhys mentioned this pull request Apr 23, 2025
Closed
kenhys pushed a commit to kenhys/fluentd that referenced this pull request Apr 23, 2025
@Watson1978 @kenhys
v1_parser: use URI#open instead of URI.open (fluent#4854)
97fd165 
**Which issue(s) this PR fixes**:
Fixes #

**What this PR does / why we need it**:
By [CodeQL
documentation](https://codeql.github.com/codeql-query-help/ruby/rb-non-constant-kernel-open/),
it is safer to avoid using `URI.open`.

This is similar with fluent#4848

**Docs Changes**:

**Release Note**:

Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
@kenhys kenhys mentioned this pull request Apr 23, 2025
Merged
daipom pushed a commit that referenced this pull request Apr 24, 2025
@kenhys @Watson1978
Backport(v1.16): v1_parser: use URI#open instead of URI.open (#4854) (#…
47b126b 
…4921)

**Which issue(s) this PR fixes**: 
Backport #4854
Fixes #

**What this PR does / why we need it**: 
By [CodeQL
documentation](https://codeql.github.com/codeql-query-help/ruby/rb-non-constant-kernel-open/),
it is safer to avoid using `URI.open`.

This is similar with #4848

**Docs Changes**:

**Release Note**:

Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Kentaro Hayashi <hayashi@clear-code.com>
Co-authored-by: Shizuo Fujita <fujita@clear-code.com>
@kenhys kenhys added the backported "backport to LTS" is done label Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daipom daipom daipom approved these changes

Assignees
No one assigned
Labels
backport to LTS We will backport this fix to the LTS branch backported "backport to LTS" is done
Projects
None yet
Milestone
v1.19.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Watson1978 @daipom @kenhys