Skip to content

Comments

Apply simple filtering of the query string passed through to real database drivers#357

Closed
rnorth wants to merge 1 commit intomasterfrom
prevent-tc-driver-param-leakage
Closed

Apply simple filtering of the query string passed through to real database drivers#357
rnorth wants to merge 1 commit intomasterfrom
prevent-tc-driver-param-leakage

Conversation

@rnorth
Copy link
Member

@rnorth rnorth commented Jun 4, 2017

Refs #345

bsideup
bsideup reviewed Jun 4, 2017
View reviewed changes
modules/jdbc/src/main/java/org/testcontainers/jdbc/ContainerDatabaseDriver.java
}

private String getQueryStringWithoutTCParams(String queryString) {
return TC_PARAM_MATCHING_PATTERN.matcher(queryString)
Copy link
Member

@bsideup bsideup Jun 4, 2017

Choose a reason for hiding this comment

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

What if we just re-create URL without TC_ query parameters?
Something like https://stackoverflow.com/a/41725109

@kiview kiview mentioned this pull request Jun 5, 2017
Closed
kiview
kiview requested changes Jun 9, 2017
View reviewed changes
modules/jdbc-test/src/test/java/org/testcontainers/jdbc/JDBCDriverTest.java
{"jdbc:tc:mysql:5.5.43://hostname/databasename", false, false, false},
{"jdbc:tc:mysql://hostname/databasename?TC_INITSCRIPT=somepath/init_mysql.sql", true, false, false},
{"jdbc:tc:mysql://hostname/databasename?TC_INITFUNCTION=org.testcontainers.jdbc.JDBCDriverTest::sampleInitFunction", true, false, false},
{"jdbc:tc:mysql://hostname/databasename?TC_INITFUNCTION=org.testcontainers.jdbc.JDBCDriverTest::sampleInitFunction&useUnicode=yes", true, false, false},
Copy link
Member

@kiview kiview Jun 9, 2017

Choose a reason for hiding this comment

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

I think this test isn't adding any verification of the actual implementation, since the tests will succeed even if I change the code in ContainerDatabaseDriver to the original implementation.

TBH I don't have a better idea to test matching and building the JDBC url but refactoring this component out into it's own class and unit testing it. If you like I can take on this work and also switch to using an URL builder like @bsideup suggested.

Copy link
Member Author

@rnorth rnorth Jun 10, 2017

Choose a reason for hiding this comment

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

Yes, fair point - I'll have a bit more of a think about it, and probably break out into a unit test.

I do have a branch for refactoring towards the Apache URL parser, but I'm not quite happy with it yet. It was the same amount of code (or actually I think a few more lines), and ends up mixing the two URL parsing approaches anyway. We probably shouldn't use both. IIRC I originally used regexes because JDBC URLs couldn't be parsed (colon in scheme part), but I could be wrong. I may have just been using the JRE URL parsing libs; perhaps the Apache library can handle it though.

I'll have a try!

@sooty1892 sooty1892 mentioned this pull request Jul 16, 2017
Closed
@rnorth rnorth mentioned this pull request Feb 3, 2018
Merged
@rnorth rnorth mentioned this pull request Feb 26, 2018
Closed
@rnorth
Copy link
Member Author

rnorth commented Mar 13, 2018

With #594 about to land I think I'll close this.

@rnorth rnorth closed this Mar 13, 2018
rnorth pushed a commit that referenced this pull request Mar 17, 2019
@dependabot @rnorth
Bump mockito-core from 2.24.5 to 2.25.0 in /core (#1294)
e326e3e 
Bumps [mockito-core](https://github.com/mockito/mockito) from 2.24.5 to 2.25.0.
<details>
<summary>Release notes</summary>

*Sourced from [mockito-core's releases](https://github.com/mockito/mockito/releases).*

> ## v2.25.0
> <sup><sup>*Release notes were automatically generated by [Shipkit](http://shipkit.org/)*</sup></sup>
> 
> #### 2.25.0
>  - 2019-03-05 - [11 commits](mockito/mockito@v2.24.10...v2.25.0) by [Szczepan Faber](https://github.com/mockitoguy) (10), Garfield Tan (1) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.25.0-green.svg)](https://bintray.com/mockito/maven/mockito/2.25.0)
>  - New API to clean up all inline mocks after test [([#1619](https://github-redirect.dependabot.com/mockito/mockito/issues/1619))](https://github-redirect.dependabot.com/mockito/mockito/pull/1619)
>  - Memory leak in mockito-inline calling method on mock with at least a mock as parameter [([#1614](https://github-redirect.dependabot.com/mockito/mockito/issues/1614))](https://github-redirect.dependabot.com/mockito/mockito/issues/1614)
>  - Cross-references and a single spy cause memory leak [([#1533](https://github-redirect.dependabot.com/mockito/mockito/issues/1533))](https://github-redirect.dependabot.com/mockito/mockito/issues/1533)
>  - Nested spies cause memory leaks  [([#1532](https://github-redirect.dependabot.com/mockito/mockito/issues/1532))](https://github-redirect.dependabot.com/mockito/mockito/issues/1532)
> 
> ## v2.24.10
> <sup><sup>*Release notes were automatically generated by [Shipkit](http://shipkit.org/)*</sup></sup>
> 
> #### 2.24.10
>  - 2019-03-05 - [4 commits](mockito/mockito@v2.24.9...v2.24.10) by [Tim van der Lippe](https://github.com/TimvdLippe) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.10-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.10)
>  - Prevent NPE in findTypeFromGenericInArguments [([#1648](https://github-redirect.dependabot.com/mockito/mockito/issues/1648))](https://github-redirect.dependabot.com/mockito/mockito/pull/1648)
> 
> ## v2.24.9
> <sup><sup>*Release notes were automatically generated by [Shipkit](http://shipkit.org/)*</sup></sup>
> 
> #### 2.24.9
>  - 2019-03-04 - [12 commits](mockito/mockito@v2.24.7...v2.24.9) by 6 authors - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.9-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.9)
>  - Commits: [Brice Dutheil](https://github.com/bric3) (5), [Tim van der Lippe](https://github.com/TimvdLippe) (3), [epeee](https://github.com/epeee) (1), [Fr Jeremy Krieg](https://github.com/kriegfrj) (1), [PaweB PamuBa](https://github.com/PawelPamula) (1), shipkit-org (1)
>  - [Java 9 support] ClassCastExceptions with JDK9 javac [([#357](https://github-redirect.dependabot.com/mockito/mockito/issues/357))](https://github-redirect.dependabot.com/mockito/mockito/issues/357)
>  - [Bugfixes] RETURNS_DEEP_STUBS causes "Raw extraction not supported for : 'null'" in some cases [([#1621](https://github-redirect.dependabot.com/mockito/mockito/issues/1621))](https://github-redirect.dependabot.com/mockito/mockito/issues/1621)
>  - Update shipkit plugin (v2.1.6) [([#1647](https://github-redirect.dependabot.com/mockito/mockito/issues/1647))](https://github-redirect.dependabot.com/mockito/mockito/pull/1647)
>  - VerificationCollector to handle non-matching args and other assertions [([#1644](https://github-redirect.dependabot.com/mockito/mockito/issues/1644))](https://github-redirect.dependabot.com/mockito/mockito/pull/1644)
>  - VerificationCollector doesn't work for invocations with non-matching args [([#1642](https://github-redirect.dependabot.com/mockito/mockito/issues/1642))](https://github-redirect.dependabot.com/mockito/mockito/issues/1642)
>  - Fix returns mocks for final classes [([#1641](https://github-redirect.dependabot.com/mockito/mockito/issues/1641))](https://github-redirect.dependabot.com/mockito/mockito/pull/1641)
>  - Removes inaccessible links from javadocs in Mockito.java [([#1639](https://github-redirect.dependabot.com/mockito/mockito/issues/1639))](https://github-redirect.dependabot.com/mockito/mockito/pull/1639)
>  - Mockito.java contains inaccessible links to articles. [([#1638](https://github-redirect.dependabot.com/mockito/mockito/issues/1638))](https://github-redirect.dependabot.com/mockito/mockito/issues/1638)
>  - Handle terminal type var with bounds [([#1624](https://github-redirect.dependabot.com/mockito/mockito/issues/1624))](https://github-redirect.dependabot.com/mockito/mockito/pull/1624)
>  - Return null instead of causing a CCE [([#1612](https://github-redirect.dependabot.com/mockito/mockito/issues/1612))](https://github-redirect.dependabot.com/mockito/mockito/pull/1612)
> 
> ## v2.24.8
> <sup><sup>*Release notes were automatically generated by [Shipkit](http://shipkit.org/)*</sup></sup>
> 
> #### 2.24.8
>  - 2019-03-01 - [2 commits](mockito/mockito@v2.24.7...v2.24.8) by [PaweB PamuBa](https://github.com/PawelPamula) (1), [shipkit-org](https://github.com/shipkit-org) (1) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.8-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.8)
>  - Removes inaccessible links from javadocs in Mockito.java [([#1639](https://github-redirect.dependabot.com/mockito/mockito/issues/1639))](https://github-redirect.dependabot.com/mockito/mockito/pull/1639)
>  - Mockito.java contains inaccessible links to articles. [([#1638](https://github-redirect.dependabot.com/mockito/mockito/issues/1638))](https://github-redirect.dependabot.com/mockito/mockito/issues/1638)
> 
> ## v2.24.7
> <sup><sup>*Release notes were automatically generated by [Shipkit](http://shipkit.org/)*</sup></sup>
> 
> #### 2.24.7
>  - 2019-02-28 - [2 commits](mockito/mockito@v2.24.6...v2.24.7) by [shipkit-org](https://github.com/shipkit-org) (1), [Tim van der Lippe](https://github.com/TimvdLippe) (1) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.7-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.7)
>  - Fix handling of generics in ReturnsMocks [([#1635](https://github-redirect.dependabot.com/mockito/mockito/issues/1635))](https://github-redirect.dependabot.com/mockito/mockito/pull/1635)
> 
> ## v2.24.6
></tr></table> ... (truncated)
</details>
<details>
<summary>Commits</summary>

- [`70c6fa4`](mockito/mockito@70c6fa4) 2.25.0 release (previous 2.24.10) + release notes updated by CI build 3956
- [`a7ee084`](mockito/mockito@a7ee084) Merge pull request [#1619](https://github-redirect.dependabot.com/mockito/mockito/issues/1619) from ttanxu/release/2.x
- [`959cba4`](mockito/mockito@959cba4) Merge branch 'release/2.x' into release/2.x
- [`1641868`](mockito/mockito@1641868) Javadoc
- [`d61a8be`](mockito/mockito@d61a8be) Added missing coverage
- [`af2d33b`](mockito/mockito@af2d33b) 2.24.10 release (previous 2.24.9) + release notes updated by CI build 3953
- [`1183d1f`](mockito/mockito@1183d1f) Merge pull request [#1648](https://github-redirect.dependabot.com/mockito/mockito/issues/1648) from mockito/TimvdLippe-patch-1
- [`2894895`](mockito/mockito@2894895) Fix checkstyle warnings
- [`2d21a5a`](mockito/mockito@2d21a5a) Add regression test
- [`b5e9400`](mockito/mockito@b5e9400) Prevent NPE in findTypeFromGenericInArguments
- Additional commits viewable in [compare view](mockito/mockito@v2.24.5...v2.25.0)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=org.mockito:mockito-core&package-manager=gradle&previous-version=2.24.5&new-version=2.25.0)](https://dependabot.com/compatibility-score.html?dependency-name=org.mockito:mockito-core&package-manager=gradle&previous-version=2.24.5&new-version=2.25.0)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
If all status checks pass Dependabot will automatically merge this pull request.

[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot ignore this [patch|minor|major] version` will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in the `.dependabot/config.yml` file in this repo:
- Update frequency (including time of day and day of week)
- Automerge options (never/patch/minor, and dev/runtime dependencies)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

</details>
@rnorth rnorth deleted the prevent-tc-driver-param-leakage branch June 18, 2020 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bsideup bsideup bsideup left review comments

@kiview kiview kiview requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rnorth @bsideup @kiview