Hacker News Reader

Comment by raggi | Hacker News Reader

You're warned:

    const messageText = inputTextBox.value.replace(/<\/?[^>]+(>|$)/g, "");

    var $newdiv = $('<div class="' + messageDivClass + '"><p>' + messageText + '<p/><div/>').css({
        'opacity': '0.95'
    });

View on HN

The safe, proper way to do this is both shorter and more readable!

Both with jQuery:

    const $newdiv = $('<div>')
      .addClass(messageDivClass)
      .text(inputTextBox.value)
      .css({'opacity': '0.95'});

and without:

    const newdiv = document.createElement('div');
    newdiv.classList.add(messageDivClass);
    newdiv.innerText = inputTextBox.value;
    newdiv.style.opacity = '0.95';

Hey, creator of the site here, thank you so much for the tip!

Unfortunately I'm quite new to full stack. ~1.5 months ago I didn't know a lick of HTML (let alone CSS, JS, any backend framework, AWS, etc), and I'm making lots of mistakes along the way. Currently trying to figure out how to improve my DB performance as Spring's default pagination seems to be quite slow and my site is getting hugged to death at the moment. If anyone has any tips for that especially I'm all ears.

At any rate, your suggestion seems easy enough to put in, so I'll put it in ASAP! Thanks again

You could also benefit from using websockets instead of sending the message to a DB and fetching it from there. This way, you'd have instant communication between the users and there's no need to store anything.

Reduce the refresh rate drastically, augment the number of messages fetched at once (let's say 200), and display them slowly. Should reduce 20 times the load on DB while making no sensible difference for end users anyway.

Ok interesting idea. My one fret here would be that it would decrease interactivity on the site. Sometimes people send messages in response to each other, right? But nonetheless I'll experiment with increasing the refresh rate and maybe strike a nicer balance

I agree with you. Realtime is the key intrigue after the site itself, people will go off it when they realise it's not live.

second this, I managed a brief conversation with someone when trying this out and it was great, a few other messages seemed to chime in as well. perf/eff can be improved as mentioned above but would lose the fun, imo

Congratulations on getting to the front page of HN and Reddit / mostly surviving the hug of death!

Sounds like you're using offset for pagination.

https://use-the-index-luke.com/no-offset

A bit more explanation for you or any other helpful people reading, I'm using Spring's PagingAndSortingRepository.findAll(Pageable) function, and then passing in sort=id,desc&size=10 via the API request params (but no explicit offset AFAIK).

Found one potentially good stackoverflow to dig into so far https://stackoverflow.com/questions/44021665/why-is-a-pagina...

Hi, is there any way to contact you privately?

I would like to use this idea on a project of mine.

Sure. Feel free to DM me on Reddit (u/VentScape). Eventually I'll set up some @ventscape.life emails to use but I haven't gone through the effort yet

Holy hell, OP.

What prevents me from just calling "postMessage" directly myself, which does nothing to prevent the "sanitation" that the replace function call there is supposed to do?

Please sanitize messages on the server instead.

The fact that it's also called when its added to the stack for display. OP should definitely still sanitize on the server, but it's not as bad as it sounds at first glance.

As an exercise I'd love it someone actually posted a payload to exploit that regex.

Sorry, would you mind explaining what this will do?

Improper input sanitation, you can potentially add raw html to other people's pages (XSS).

Doesn't the regex sanitize it before display?

Yes but it's pretty easily bypassed

I asked earlier because I'm genuinely curious, but do you have a payload that would bypass this particular regex?

How is it easily bypassed?

That's terrible. OP should take the site down and fix those issues.

Yeah, go ahead and down vote without explaining yourself. It's so educational!

My guess is that you were downvoted because you sound like you are talking about a very important system with a huge number of users that could have catastrophic consequences if the vulnerability is not fixed. It's just a project from a novice programmer having fun. Hence, no need to take the site down.

So it's not a principled decision? How many users does the site have? 10000, 100000? To me, that's a lot. Thanks for answering.

What are you warning about?

XSS vulnerabilities

Reply on news.ycombinator.com