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

fix(id): ascii-only identify versions #9465

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lidel
Copy link
Member

@lidel lidel commented Dec 6, 2022

This is a follow-up for #9457 that ensures we trim identify versions to ASCII-only, max 64 characters.

It is applied to both values sent by Kubo, and ones read from other peers.

I also submitted libp2p/specs#491 to make this canonical behavior across implementations.

@lidel lidel requested review from guseggert and Jorropo December 6, 2022 20:07
@lidel lidel self-assigned this Dec 6, 2022
@lidel lidel force-pushed the fix/agent-version-suffix branch from 6357a06 to 0d30552 Compare December 6, 2022 20:26
@lidel lidel force-pushed the fix/agent-version-suffix branch from 0d30552 to 7384cb4 Compare December 6, 2022 20:35
@lidel lidel marked this pull request as draft December 6, 2022 20:49
@lidel lidel force-pushed the fix/agent-version-suffix branch 2 times, most recently from 57cb269 to a593889 Compare December 6, 2022 21:30
@lidel lidel force-pushed the fix/agent-version-suffix branch from a593889 to 2ad8bf7 Compare December 6, 2022 21:35
@lidel lidel marked this pull request as ready for review December 6, 2022 22:55
@marten-seemann
Copy link
Member

I'm not sure I understand the motivation behind this. I'd prefer to embrace UTF-8, instead of taking extra steps to restrict ourselves to ASCII.

@BigLep BigLep marked this pull request as draft December 13, 2022 17:42
@BigLep BigLep mentioned this pull request Jan 3, 2023
@BigLep
Copy link
Contributor

BigLep commented Mar 9, 2023

@lidel : FYI I bumped this to 0.21 (not 0.20). Feel free to pull it in sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: 🔎 In Review
Development

Successfully merging this pull request may close these issues.

3 participants