Hot answers tagged

15

IP Detection. You are confusing an IP address with an HTTP header, the latter being utterly unreliable. It is ridiculously simple to fake an HTTP header. There is an old cautionary tale by Anthony Ferrara about how he unintentionally hacked Stack Overflow by manipulating headers. You may want to read about it. Not to mention that even if you catch one of ...


13

There's a glaring problem with this: no user input validation. A basic test revealed this: Ah, but that's not Java you say? True, but what if some poor sod has a stray HTML tag in their docstring? All HTML that's already in the user input should probably be made harmless. Either escape it, wrap it in a container that will treat it as a string instead of ...


11

For a beginner this looks like a great start! Below are some suggestions to clean things up and make things more consistent. It is a common convention for HTML to not contain spaces between attribute keys and values. So instead of <script language = "javascript" type = "text/javascript" src = "highlightSyntax.js"></...


10

Are there any problems in it or is it well-written? Well, it is not really written in a simple manner - it is very redundant. A commonly accepted principle is the Don't Repeat Yourself. principle. For example, the following block: if (getenv('HTTP_CLIENT_IP')) $mainIp = getenv('HTTP_CLIENT_IP'); else if(getenv('HTTP_X_FORWARDED_FOR')) ...


10

Does this open up the possibility for any buggy behavior that I don't know about? It's not likely to. The if statement is fine, though it can be made cleaner: Rather than selecting the element twice (once to check to see if it exists, another time to call addEventListener on it), save it in a variable: const button = document.getElementById('button01'); if ...


10

Your project looks nice, and the design of your web page is good. There were a few points I spotted with the code that are well worth taking note of. Security While security may sound less important if you're the only one using the project, the issues below are both critical for any public-facing website, and affect the correctness of the program as pointed ...


10

Security Security is super important. In 2020 it is not acceptable to write code that is blatantly vulnerable to SQL injection. Start taking good habits today. Even if you are lazy and you are the only one using this application you should still fix the SQL injection vulnerabilities because your code will choke on single quotes - and single quotes in family ...


8

Interpreting any source code language entirely by regular expression — which is to say, without actually parsing the code and building an understanding of it on a syntactic level — is notoriously difficult to do. Your regular expressions do fall prey to some of the common issues with regexp-as-parser, since it will mis-highlight all of the following: public ...


7

It looks pretty good to me. I can only see a few things to consider: More precise propertyName check You have if(e.propertyName.includes('flex')) { because Safari uses flex and others use flex-grow. Are you sure that the flex substring won't be present in any other possible CSS transitions? Even if you're sure, will readers of the code be sure? I'd change to ...


7

I've noticed that you are using Bootstrap 3.3.6, which isn't maintained anymore. The latest 3.x version is 3.4.1, and you should consider using it. However, you're doing something wrong: You are misusing the framework Below is a copy part of the HTML, with all the CSS: body { font-family: Ubuntu Mono; background-color: #F2F1F0; } .form-control {...


6

UI First thing to look at would probably the ease of use of the webpage itself. You could start by using typed input controls, like time, ie: <input type="time" step="00:30" value="10:00" id="startTime" /> Those would still render as text inputs for any browser that might not be able to render it, but it would ...


6

First and foremost, regardless of any real or imaginary security issue, this code is a pain in the eye. It's nearly impossible to get the gist of it and to answer the actual question because the code is constantly shifting out of sight and the vast amount of HTML gets in the way in the midst of the supposedly email verification code. Useless verifications ...


6

I am concerned that you are not digesting the advice given in previous reviews, because you are ONLY to call htmlspecialchars() just before you print to an html document (print to screen). Effectively, you only need to handle whitespace related characters. Since you seem to be concerned with handling multibyte characters, the unicode flag should be used on ...


6

Variables Always use a variable declaration when first declaring your variable. If you don't your variable will be put into the global scope, which is almost never what you want. It also makes it more clear that you don't want to re-assign to an already existing global variable. So for starting_day, for example, put a var, let, or const in front of it. HTML ...


6

Welcome to Code Review. The CSS looks quite good. I haven’t seen too many style sheets with color lightsalmon 😃. Below are some suggestions. Cache DOM references ”...DOM access is actually pretty costly - I think of it like if I have a bridge - like two pieces of land with a toll bridge, and the JavaScript engine is on one side, and the DOM is on the ...


5

Yes, It can be better in many ways just replace all your code with this: The first thing I did is made the code formatting just a bit better. then I added a title tag a webpage cannot go on without a title after that I include the meta tags for the UTF-8 encoding and then the viewport meta tag for responsiveness. I also corrected your opening body tag you ...


5

CSVReader Reading a CSV file can be a complex task. While many CSV files are just comma-separated-values, if a value contains a comma, it would be surrounded by double quotes, and if the value contains double quotes, the double quotes themselves are doubled. To handle these more that just a basic CSV files, you really should use a CSV library, such as ...


5

Simplifying overall layout While I haven't gone through the coursera videos I did look at the syllabus. I see that the next assignment does involve using Bootstrap framework. Since this assignment requires no frameworks to be used, one option to consider looking at is flexbox. It was designed to eliminate the need for floating elements. One important thing ...


5

setAttribute? When assigning properties to elements, I'd prefer to use dot notation assignment instead of setAttribute - it's more concise and a bit easier to read and write. For example: selectElement.setAttribute("id", "RoomMenu"); selectElement.setAttribute("class", "form-control mb-4"); can turn into selectElement....


5

Unnecessary information disclosure Don't do this: $headers .= "X-Mailer: PHP". phpversion() ."\r\n" ; This will show up in the mail headers and be visible by the recipient. It's nobody's business what version of PHP you're running. In case you're running a version of PHP that has vulnerabilities, this provides hackers with insight into ...


5

Prefer CSS rules over JS It helps to separate concerns by putting CSS rules as much in the CSS file as possible - putting lots of rules in the JS can detract from the code's logic, which is likely what one is more worried about. Here, rather than assigning to style properties when the game starts, consider adding a class to the container only, and having CSS ...


5

As you are as you say "brand new" to coding this will be less like a review and more like some pointers and advice.. Some pointers Rather than define the functions as arrow functions within the class verticalScore = (score, col) => { Use the shorter syntax verticalScore(score, col) { There is a technical reason due to the way this is associated ...


4

Mutating built-in prototypes is not a good idea. It's not only inelegant, it can also lead to conflicts when other scripts on the page expect the prototypes to be unmutated. (It can even damage future attempts to integrate functionality officially, if enough sites use the bad code.) Instead of using Element.prototype, create your own class, and put methods ...


4

Indent your code! Naming convention Only use English paivitys -> update -> though really this should be with a subject, so updateObjekt? dataa -> data arvvoja -> values paivitaselekt -> updateSelection Dont mispel ssellecct -> selectElement oppttion -> option Use lowerCamelCase infoarray -> infoArray chosenid -> chosenId choseninfo -> chosenInfo ...


4

To handle such dynamic cases, you need to have dynamic code in both ts and html side.Take a look at this demo code which can be refactored furthur. Here we have changed HTML <ul> <ng-container *ngFor="let page of pages"> <li *ngIf="page.showDOM"> <a routerLink="page.routing" routerLinkActive="...


4

Program looks well structured, so this is gonna be nitpicky function isDiagonalWinner(id) { posGradient = true; Since posGradient hasn't been declared anywhere, this will be treated as a global on window['posGradient']. let would suit you nicely here. var matches = document.querySelectorAll("#board .row .elem .clickable"); The value of ...


4

Mast mentioned that it is not possible to have html-tags in for example comments, without destroying the formatting. I've solved this problem by replacing & and <: text = text.replace(new RegExp("&", "g"), "&amp;"); text = text.replace(new RegExp("<", "g"), "&lt;"); You can ...


4

I'm glad to see you incorporated much of the feedback from the answers to your previous post. I see you added the colors into CSS, but dynamically. I believe those styles for the colors could exist with the static CSS. Actually the whole addStyles() function could likely be eliminated using conditional classes (e.g. if checkbox1 is checked, then add an extra ...


4

Speaking about security, there are several issues: As a rule the attacker will go the easy way: send and analyze first request, remember exerciseText and solution, then send an unlimited number of requests from an unlimited number of devices. So, at least, you should use a timestamp to shorten the token's lifetime and tie it to a specific IP. In fact your ...


Only top voted, non community-wiki answers of a minimum length are eligible