Microtask 1#1
Conversation
tgr
left a comment
tgr
left a comment
There was a problem hiding this comment.
This PR doesn't contain most of the changes. You can always make a PR from the master into a new branch (instead of the other way around) if you don't want to rewrite master's history.
|
@tgr Sir, I have updated the PR to contain all the changes. |
| Comment = {{x.comment}} <br> | ||
| Size = {{x.size}} <br> | ||
|
|
||
| <a href="https://en.wikipedia.org/w/index.php?title=User:{{x.user}}&diff=prev&oldid={{x.revid}}"> |
There was a problem hiding this comment.
Technically, that's not valid HTML (which is why Github shows it in red); special characters like & or " need to be encoded in attributes. So it should be {{x.user}}&diff. In practice, it works anyway and no one really cares about, so feel free to ignore :) just mentioning for sake of completeness.
| {% if userid %} | ||
| Username = {{username}} <br> | ||
| Userid = {{userid}} <br> | ||
| <a href="https://phabricator.wikimedia.org/p/{{username}}/"> Profile link </a> |
There was a problem hiding this comment.
The Phabricator username is not typically the same as the wiki username.
There was a problem hiding this comment.
Alright. I added that link to augment to the information. Since I just got to know that phabricator and wiki usernames are not same, I'd rather remove this link. Thank you .
| if(request.method=='POST'): | ||
| usr = request.POST['username'] #Here, we get the useranme fetched from html page | ||
| URL = "https://en.wikipedia.org/w/api.php?action=query&format=json&list=usercontribs&ucuser=" | ||
| URL = URL + usr |
There was a problem hiding this comment.
Handled the situation. 👍
| URL = "https://en.wikipedia.org/w/api.php?action=query&format=json&list=usercontribs&ucuser=" | ||
| URL = URL + usr | ||
| r = requests.get(URL) #Recieved data in json-format | ||
| lst = json.loads(r.text) |
There was a problem hiding this comment.
The request might fail (e.g. the API is down and responds with HTTP 503), in which case this won't work.
There was a problem hiding this comment.
Ok. Will handle this, when response code is other than 200.
| URL = URL + usr | ||
| r = requests.get(URL) #Recieved data in json-format | ||
| lst = json.loads(r.text) | ||
| sub_lst = lst['query']['usercontribs'] #Contribution data extracted |
There was a problem hiding this comment.
The API might respond with an error, in which case this key might not exist. (You can test with a username containing <.)
| fetched_user = sub_lst[0]['user'] | ||
| fetched_user_id = sub_lst[0]['userid'] | ||
| except: | ||
| fetched_user = fetched_user_id = 'No user found' |
There was a problem hiding this comment.
Or possibly the user was found but has no edits.
There was a problem hiding this comment.
okay , this was a loophole, will handle it 👍
|
@tgr I have made the required changes . Please review. |
| lst = json.loads(r.text) | ||
| try: | ||
| if(lst['error']): | ||
| return(render(request,'App/index.html',{"error":'INVALID INPUT'})) |
There was a problem hiding this comment.
There might be other reasons for getting an API. You probably want to show the user the error message returned by the API instead.
| def main(request): | ||
| if(request.method=='POST'): | ||
| usr = request.POST['username'] #Here, we get the username fetched from html page | ||
| usr = usr.replace("&","%26") |
There was a problem hiding this comment.
But now the username will look weird when you print it out :)
Best practice is to sanitize close to the input, escape close to the output. (Sanitize = reject invalid input. Here the API takes care of that so you don't have to. Escape = convert the variable to a safe format. "safe" depends on context (you escape differently for text in a HTML document, for an URL part, for a Javascript variable...) so it has to be done when you already know how you'll use it; ideally in the template. Most templating languages (including Django templates) support that via some special notation.
There was a problem hiding this comment.
I have tried to make the variable safe, by escaping &. Is It ok?
| if(request.method=='POST'): | ||
| usr = request.POST['username'] #Here, we get the username fetched from html page | ||
| usr = usr.replace("&","%26") | ||
| usr = usr.replace("&","") |
There was a problem hiding this comment.
Sir so should I consider input containing "&" as invalid , not deal with trying correct the handle ? Please tell . Currently , if I enter a username ey bd&808 , it converts to bd808 and displays the result.
There was a problem hiding this comment.
bd&808 is a different (valid) username though so that would prevent that person from using your tool. The percent-encoding approach wasn't wrong, you just did it at the wrong place (also, no point in limiting it to just &). See my earlier comment - you should do escaping in the template, and use whatever tools the templating language provides.
There was a problem hiding this comment.
Sir, in django templating, escaping is done by default on. Now, I have used 'safe' filter in my html file , so the username will not be auto-escaped. Am I going right now ?
|
@tgr Please review this PR. |
| {% endif %} | ||
| {% if username %} | ||
| Username = {{username}} <br> | ||
| Username = {{username | safe}} <br> |
There was a problem hiding this comment.
This turns off escaping. There is no reason to do that and it will result in some usernames looking wrong (if they contain a HTML entity). It could also be a security hole although here it probably won't be since MediaWiki disallows <> in usernames.
Default autoescaping works well when you just want to display text. Where it doesn't work is when you want use a variable in an URL or a Javascript variable or any other context where the escaping/encoding rules are different from that of HTML.
There was a problem hiding this comment.
True, I understood not this thing in the first instance. Marking it safe is not the solution. So I have handled this now, using urlencoding. Now, the code works for all possible usernames else displays the error msg.
|
@tgr Sir, Please review, sorry for delay. |
tgr
left a comment
tgr
left a comment
There was a problem hiding this comment.
Looks quite nice! The edit links use the wrong title parameter, but MediaWiki ignores the title anyway when oldid is set, so it doesn't matter much.
| return render(request, "App/index.html", {"error": "Please enter a username"}) | ||
|
|
||
| # if username is not blank | ||
| params = {'ucuser': username} |
There was a problem hiding this comment.
It would be more readable to put the constant parameters here as well.
2 participants
@tgr Please review