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

Handle messages with poorly formatted children #120

Closed
guusdk opened this issue May 18, 2024 · 15 comments
Closed

Handle messages with poorly formatted children #120

guusdk opened this issue May 18, 2024 · 15 comments
Assignees
Labels
enhancement New feature or request question Further information is requested
Milestone

Comments

@guusdk
Copy link

guusdk commented May 18, 2024

Describe the bug

I'm creating an Openfire plugin that wraps xmpp-web and deploys it in Openfire's embedded web server. This will make deployment of your client into Openfire super simple.

While testing the plugin, and logging in with my own account (lots of roster items, lots of MUC rooms to join), the console log is full of stace traces like this one:

Uncaught TypeError: Cannot read properties of undefined (reading 'xmlns')
    at index-0eca49fe.js:15:268250
    at ol.getChildrenByFilter (index-0eca49fe.js:15:237771)
    at xk.parseMessage (index-0eca49fe.js:15:268219)
    at mh.parseStanza (index-0eca49fe.js:15:266731)
    at Ye.emit (index-0eca49fe.js:15:228445)
    at mh._onElement (index-0eca49fe.js:15:245001)
    at Ye.emit (index-0eca49fe.js:15:228386)
    at O2.onEndElement (index-0eca49fe.js:15:251633)
    at Ye.emit (index-0eca49fe.js:15:228386)
    at e2._handleTagOpening (index-0eca49fe.js:15:239931)

With my (very) limited javascript skills, I tried clicking through to the source code, which pointed at this line of code:

image

It appears that this bit of code expectes there always to be an attrs property on whatever is being iterated over there?

I'm certainly not ruling out a bug in Openfire here, but it might be good to perform a null-check on attrs before trying to access one of its properties.

Steps to reproduce

No response

Expected behavior

I'm certainly not ruling out a bug in Openfire here, but it might be good to perform a null-check on attrs before trying to access one of its properties.

Relevant log

No response

local.js configuration

No response

XMPP-web version

0.10.0

Installation

Github release archive

XMPP server(s)

Openfire

Browser(s)

Chrome

Device(s)

No response

Other information

No response

@guusdk guusdk added the bug Something isn't working label May 18, 2024
@nioc
Copy link
Owner

nioc commented May 20, 2024

This is strange behavior. It implies that some XMPP messages exchanged do not include namespace attribute 😲.

Could you post the XMPP messages concerned (from the browser's network>websocket tab)?
image

<open xmlns='urn:ietf:params:xml:ns:xmpp-framing' version='1.0' xml:lang='en' id='4aaae241-3906-4e7f-b519-a4073cf5bda2' from='mydomain.ltd'/>

@nioc nioc added the question Further information is requested label May 20, 2024
@guusdk
Copy link
Author

guusdk commented May 20, 2024

If I'm reading this correctly, then the data that's being parsed is a newline character only?

image

@nioc
Copy link
Owner

nioc commented May 20, 2024

Mmmhh I do not know as I usually do not use browser debugger with compiled js 😆
What messages do you see in Network tab (on the /xmpp-websocket websocket)?

This function should not be called with anything else other than a XML string. Does your plugin send HTML code (of this app) toward the websocket?

@guusdk
Copy link
Author

guusdk commented May 20, 2024

There's to much data for me to go through it manually. I'm joined a good deal of chatrooms. Most of them will send message history. I've got about 100 contacts, many of them will send presence. These errors are caught during the initial part of the session (right after logging in) so I do assume it has to do with MAM or presence, but I'm not sure.

The app is hosted at https://xmpp.igniterealtime.org:7483/xmppweb

I'd be happy to provide you with an account on that server, so that you can debug on your own end.

@nioc
Copy link
Owner

nioc commented May 20, 2024

Okay I understand, you can provide me a test account, I'll have a look.

Another option: I can provide you a debug version on this thread, if you have the possibility to install it in parallel (another Nginx path) we'll be able to do a more accurate error try catch and see the problematic frame.

@nioc nioc self-assigned this May 20, 2024
@guusdk
Copy link
Author

guusdk commented May 20, 2024

I have created an account for you, nioc / zL9AhvcASPghiGW - I have added myself to the roster of that account. Please change the password when you first login, or contact me to change it for you.

Sadly, using this account, I've not been able to reproduce the error yet.

@guusdk
Copy link
Author

guusdk commented May 20, 2024

Debug output

image

@guusdk
Copy link
Author

guusdk commented May 20, 2024

Similar issue here:
image

@nioc
Copy link
Owner

nioc commented May 20, 2024

Summary of our exchange:

  • I can not reproduce this issue,
  • It seems that your account is loading old messages whose children have no xmlns attribute,
  • Sadly, this is acceptable from the point of view of the XMPP standard,
  • I fixed some lines, and then sent you a version with all lines fixed.

Let me know if this version fixes all cases of this problem.

@nioc nioc added enhancement New feature or request and removed bug Something isn't working labels May 20, 2024
@nioc nioc changed the title Uncaught TypeError: Cannot read properties of undefined (reading 'xmlns') Handle messages with poorly formatted children May 20, 2024
@nioc nioc added this to the 0.10.2 milestone May 20, 2024
@guusdk
Copy link
Author

guusdk commented May 21, 2024

I have tested the latest debug build that you left for me last night. It appears to fix the problem!

@nioc
Copy link
Owner

nioc commented May 21, 2024

Great news, I'll release it tonight.

@nioc nioc closed this as completed in 46c34db May 21, 2024
@guusdk
Copy link
Author

guusdk commented May 26, 2024

Thanks!

We've now created an Openfire plugin for this client: https://discourse.igniterealtime.org/t/new-openfire-plugin-xmpp-web

@nioc
Copy link
Owner

nioc commented May 26, 2024

Thanks, it is very kind.

I just have a question about maintenance: How does the plug-in handle web-xmpp updates? I mean, does it has to be updated for each future release or it check/use the latest release at installation/start?

@guusdk
Copy link
Author

guusdk commented May 26, 2024

Every update is manual. We do the same for a couple of other web clients. It is a bit of a hassle, but not to bad.

@nioc
Copy link
Owner

nioc commented May 26, 2024

Ok 😉
Thanks for it.
If you want to write a little something in the project README for advertising let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants