9 ms·
You're warned: const messageText = inputTextBox.value.replace(/<\/?[^>]+(>|$)/g, ""); var $newdiv = $('<div class="' + messageDivClass + '"><p>' + mes
by raggi 6y ago
You're warned:
const messageText = inputTextBox.value.replace(/<\/?[^>]+(>|$)/g, "");
var $newdiv = $('<div class="' + messageDivClass + '"><p>' + messageText + '<p/><div/>').css({
'opacity': '0.95'
});
- szhu 6y agoThe 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';
- VentScape 6y agoHey, 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
- eindiran 6y agoCongratulations on getting to the front page of HN and Reddit / mostly surviving the hug of death!
- andreareina 6y agoSounds like you're using offset for pagination. https://use-the-index-luke.com/no-offset https://use-the-index-luke.com/no-offset
- VentScape 6y agoA 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-paginated-query-slower-than-a-plain-one-with-spring-data https://stackoverflow.com/questions/44021665/why-is-a-pagina...
- C4stor 6y agoReduce 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.
- VentScape 6y agoOk 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
- corobo 6y agoI agree with you. Realtime is the key intrigue after the site itself, people will go off it when they realise it's not live.
- nmeta 6y agosecond 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
- jlis 6y agoYou 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.
- simply_sam 6y agoHi, is there any way to contact you privately? I would like to use this idea on a project of mine.
- VentScape 6y agoSure. 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
- gitrog 6y agoSorry, would you mind explaining what this will do?
- thepete2 6y agoImproper input sanitation, you can potentially add raw html to other people's pages (XSS).
- madhato 6y agoDoesn't the regex sanitize it before display?
- some_random 6y agoYes but it's pretty easily bypassed
- halvo 6y agoWhat are you warning about?
- mxxx 6y agoXSS vulnerabilities
- llarsson 6y agoHoly 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.
- Mapsmithy 6y agoThe 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.
- vfinn 6y agoThat's terrible. OP should take the site down and fix those issues.
- vfinn 6y agoYeah, go ahead and down vote without explaining yourself. It's so educational!
- SwiftyBug 6y agoMy 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.
- vfinn 6y agoSo it's not a principled decision? How many users does the site have? 10000, 100000? To me, that's a lot. Thanks for answering.