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

Do we care about address length vs message length in circuit-relay? #29

Open
dryajov opened this issue Jul 16, 2017 · 6 comments
Open

Comments

@dryajov
Copy link
Member

dryajov commented Jul 16, 2017

Now that we have a proper protobuf spec for the circuit-relay messages, do we care about individual addresses length? It seems like we should restrict the overall size of the protobuf message, but other than that, checking for address length doesn't seem to add any value anymore. @vyzo proposed 4096 as an arbitrary message size, which I think is reasonable.

Given this, lets remove the address length restriction in favor of a message size.

@diasdavid @whyrusleeping @lgierth

@dryajov
Copy link
Member Author

dryajov commented Jul 17, 2017

This should also eliminate the need for the ADDRESS_TO_LONG error codes, but possibly introduces a new one - MSG_TOO_LARGE

edit: typo

@vyzo
Copy link
Contributor

vyzo commented Jul 17, 2017

Fwiw the Go implementation I am working on does not emit any ADDRESS_TOO_LONG messages, because they are not relevant anywhere.

For the max message size, it's not explicitly tested either.
A delimited reader is created, and if that fails to parse the message it currently closes the connection, although it would certainly be nicer to emit an error message.
Perhaps we want a MALFORMED_MSG error instead of MSG_TOO_LARGE?

@jvican
Copy link

jvican commented Aug 11, 2017

I think a MALFORMED_MSG is a better candidate, especifically if it can capture the reason why it fails: either because the message is too large, there is an encoding issue, etcetera.

@dryajov
Copy link
Member Author

dryajov commented Aug 16, 2017

@jvican we're not currently sending error messages back, just codes - hence the current 200 vs 300 error code ranges, they signal which leg of the relay broke.

@whyrusleeping
Copy link
Contributor

I think MSG_TOO_LARGE is a good message, though its a fair argument that MALFORMED_MSG is equivalent, given that the most common cause of a "too long" message is that its data is all screwed up. That said, messages could be malformed for other reasons, but have a valid length. I'm fine with either choice.

@mxinden
Copy link
Member

mxinden commented Mar 26, 2021

This is addressed in the current circuit relay v2 design (libp2p/go-libp2p-circuit#125). Unless anyone objects this can be closed once circuit relay v2 lands in the spec repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triage
Development

No branches or pull requests

5 participants