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

1.20.4 Hover Event Sanitization #3675

Open
4 tasks done
valaphee opened this issue Jan 30, 2024 · 5 comments
Open
4 tasks done

1.20.4 Hover Event Sanitization #3675

valaphee opened this issue Jan 30, 2024 · 5 comments
Assignees

Comments

@valaphee
Copy link
Contributor

valaphee commented Jan 30, 2024

/viaversion dump Output

Latest commit of ViaVersion 0d788b0

Console Error

Not applicable

Bug Description

With 1.20.3+ and the usage of nbt for chat components, the entity hover event's type identifier seems to require a lower case but servers running any other version could still use upper case identifiers which would disconnect any client newer than 1.20.3.

https://i.imgur.com/jEfTRyi.png
https://i.imgur.com/aPoG3ja.png
https://i.imgur.com/4ZcYN8Z.png

Steps to Reproduce

  1. Send a message with a hover event (/tellraw)
  2. Observe the clients behavior in 1.20.2 vs 1.20.3+

Expected Behavior

The hover event should work properly and not cause a disconnect.

Additional Server Info

Paper 1.8.9

Checklist

  • Via plugins are only running on EITHER the backend servers (e.g. Paper) OR the proxy (e.g. BungeeCord), not on both.
  • I have included a ViaVersion dump.
  • If applicable, I have included a paste (not a screenshot) of the error.
  • I have tried the latest build(s) from https://ci.viaversion.com/ and the issue still persists.
@Barvalg
Copy link
Member

Barvalg commented Jan 30, 2024

Error: Invalid or missing dump

@valaphee
Copy link
Contributor Author

valaphee commented Jan 30, 2024

Its also weird that mcstructs doesn't catch this error as there is a check that the name has to be lc, but it somehow passes through.

But there should also be a more sophisticated conversion for hover event entity type, as for example pigman -> piglin conversion is also not applied, etc.

@MrTommyt
Copy link

Any ETA on when this problem would be fixed? I've seen other issues that can fall under this one already opened and they've all been opened from long ago, is this being planned to be fixed anytime soon?

@prxgma
Copy link

prxgma commented Apr 12, 2024

Do we have an ETA on this yet? Or just any update on this further? I am experiencing issues with this as well and it's been a rather consistent and disruptive issue.

@FlorianMichael
Copy link
Member

FlorianMichael commented Apr 12, 2024

@valaphee The reason MCStructs doesn't catch this error is that this is nothing related to the structure of the component, MCStructs doesn't validate components for their validation in terms of the client's ability to read them, it only validates the structure of a component. VV is missing string id/name conversions in components in general, this also applies to show_item where the id tag is never converted causing items to be invalid in most version ranges. I'll look at this issue next time I work on components once I am done with some other stuff.

@FlorianMichael FlorianMichael self-assigned this Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants