Skip to content
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.

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

IndexKeys is run on pre-canonicalized entries #854

Closed
asraa opened this issue Jun 1, 2022 · 2 comments · Fixed by #967
Closed

IndexKeys is run on pre-canonicalized entries #854

asraa opened this issue Jun 1, 2022 · 2 comments · Fixed by #967
Labels
bug Something isn't working

Comments

@asraa
Copy link
Contributor

asraa commented Jun 1, 2022

Description

Version

The flow of creating a log entry is curious to me:

func createLogEntry(params entries.CreateLogEntryParams) (models.LogEntry, middleware.Responder) {

  1. A new entry is created based on the parameter proposed entry.
  2. A leaf node is created by canonicalizing the entry
  3. The original entry calls index keys.

In many entry types, logic is performed in Canonicalize to set up the entry, and it's not clear that IndexKeys does not have access to the canonicalized entry fields. This is also a problem if Canonicalize does some operation that may modify IndexKeys.

I realized this was the root cause to the bug that @priyawadhwa fixed from #800 where assumed that intoto entries populated the Content Hash (but only canonicalized ones did).

Shouldn't IndexKeys operate on the canonicalized entry?

@asraa asraa added the bug Something isn't working label Jun 1, 2022
@asraa
Copy link
Contributor Author

asraa commented Jun 1, 2022

From @haydentherapper 👍

"Maybe we need to figure out how to do canonicalization very early on in the lifecycle of a request

so nothing ever operates on a non-canonical entry"

@dlorenc
Copy link
Member

dlorenc commented Jun 15, 2022

I'm not sure this is doable since we strip a lot of information during canonicalization, on purpose. It means the index entries aren't always verifiable or repairable, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants