Skip to content

BugFix: prevent evil insertions into metadata.json resulted in corrupted package and inability to install - #8532

Merged
lasote merged 1 commit into
conan-io:developfrom
SSE4:avoid_evil_insertions
May 27, 2021
Merged

BugFix: prevent evil insertions into metadata.json resulted in corrupted package and inability to install#8532
lasote merged 1 commit into
conan-io:developfrom
SSE4:avoid_evil_insertions

Conversation

@SSE4

@SSE4 SSE4 commented Feb 19, 2021 •
edited by czoido
Loading

Copy link
Copy Markdown
Contributor

#TAGS: slow

see #8519

Changelog: BugFix: Prevent evil insertions into metadata.json resulted in corrupted package and inability to install.
Docs: omit

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

@SSE4 SSE4 closed this Feb 19, 2021
@SSE4 SSE4 reopened this Feb 19, 2021
memsharded
memsharded requested changes Feb 19, 2021
View reviewed changes

@memsharded memsharded left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like the idea to protect against this, but it seems that this should be captured earlier to redirect the flow earlier.

Comment thread conans/client/graph/graph_binaries.py Outdated
jgsogo
jgsogo approved these changes Feb 19, 2021
View reviewed changes

@jgsogo jgsogo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Safer than it was.

Comment thread conans/test/functional/command/install/install_update_test.py Outdated
memsharded
memsharded approved these changes Feb 19, 2021
View reviewed changes

@memsharded memsharded left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is good, and should be low risk.

Recall this is targeted at release/1.32, we want it in 1.33 (and of course develop)

@jgsogo

jgsogo commented Feb 26, 2021

Copy link
Copy Markdown
Contributor

We are waiting for the error to happen again in ConanCenter. We added a command just at the moment of entering a workspace to list all the files in it. So far, the workspace is always empty, and AFAIK the error hasn't happened again.

Heisenbug hunting 🐛

image

@jgsogo jgsogo self-assigned this Feb 26, 2021
@jgsogo

jgsogo commented May 13, 2021

Copy link
Copy Markdown
Contributor

We show the error again, it happened as expected: #8519 (comment)

I will add this to a release so it doesn't get lost. This patch is a step forward, for sure.

@jgsogo jgsogo assigned memsharded and unassigned jgsogo May 13, 2021
@jgsogo jgsogo added this to the 1.37 milestone May 13, 2021
lasote
lasote suggested changes May 24, 2021
View reviewed changes

@lasote lasote left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please change the changelog line to something more useful

Comment thread conans/test/functional/command/install/install_update_test.py Outdated
@SSE4 SSE4 changed the title BugFix: avoid evil insertions BugFix: prevent evil insertions into metadata.json resulted in corrupted package and inability to install May 24, 2021
SSE4
SSE4 commented May 24, 2021
View reviewed changes
Comment thread conans/test/functional/command/install/install_update_test.py Outdated
lasote
lasote reviewed May 25, 2021
View reviewed changes
Comment thread conans/test/functional/command/install/install_update_test.py Outdated
@SSE4
SSE4 force-pushed the avoid_evil_insertions branch from 23e6ffe to 6f54798 Compare May 25, 2021 10:39
@SSE4
SSE4 changed the base branch from release/1.32 to develop May 25, 2021 10:44
@SSE4
SSE4 force-pushed the avoid_evil_insertions branch from 6f54798 to 6b9d6f6 Compare May 25, 2021 10:45
@SSE4
- avoid evil insertions
94d20d5 
Signed-off-by: SSE4 <tomskside@gmail.com>
@SSE4
SSE4 force-pushed the avoid_evil_insertions branch from 6b9d6f6 to 94d20d5 Compare May 25, 2021 11:13
@lasote
lasote merged commit 7fd13bc into conan-io:develop May 27, 2021
@czoido czoido mentioned this pull request May 28, 2021
Merged
@jgsogo jgsogo mentioned this pull request Jun 30, 2021
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@memsharded memsharded memsharded approved these changes
+2 more reviewers
@jgsogo jgsogo jgsogo approved these changes
@lasote lasote lasote approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@memsharded memsharded

Labels

None yet

Projects

None yet

Milestone

1.37

Development

Successfully merging this pull request may close these issues.

4 participants

@SSE4 @jgsogo @lasote @memsharded