Skip to content

Added html files#3

Open
vlppz wants to merge 4 commits into
Poison-Berries:masterfrom
vlppz:master
Open

Added html files#3
vlppz wants to merge 4 commits into
Poison-Berries:masterfrom
vlppz:master

Conversation

@vlppz

@vlppz vlppz commented May 1, 2021

Copy link
Copy Markdown

Hello, Nikolay! I have translated the pages into html files as you requested. And made some changes to the code. I couldn't use the static _file method because that's not what it is for, but instead I used normal file manipulation.

@kolayne kolayne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, Vladimir! Thank you for your Pull Request. Sorry for such a long delay.
The code quality got much better, however, I have found some issues which I ask you to fix. Besides what I have mentioned in the files, here is what I want to ask/mention additionally:

  1. Why can/should we not use static_file for serving HTML files? I have googled a bit, and found several StackOverflow answers suggesting to do exactly that.
  2. You might want to review your commits history and give your commits appropriate names and descriptions. If you don't, I will have to squash all your changes into one commit.

Comment on lines +8 to +11
<link rel="stylesheet" type="text/css" href="/styles.css">
<link rel="preconnect" href="https://fonts.gstatic.com">
<link href="https://fonts.googleapis.com/css2?family=Comfortaa:wght@300&display=swap" rel="stylesheet">
<link href="https://fonts.googleapis.com/css2?family=Balsamiq+Sans:ital@1&display=swap" rel="stylesheet">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be in the <head>: https://www.w3schools.com/tags/tag_link.asp (same for the other HTML files)

<link href="https://fonts.googleapis.com/css2?family=Comfortaa:wght@300&display=swap" rel="stylesheet">
<link href="https://fonts.googleapis.com/css2?family=Balsamiq+Sans:ital@1&display=swap" rel="stylesheet">
</body>
</html> No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, let's keep empty lines at the end of files (same for the other files!)

<body>
<h1>Jake is so lonely!</h1>
<p>To get the password, send the letter to Jake by POST.</p1>
<br></br>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The <br> tag should not be closed: https://www.w3schools.com/tags/tag_br.asp

Suggested change
<br></br>
<br>

(same for other HTML files!)

<title>Cookies Tasks</title>
</head>
<body>
<h1 class=".header1">Select a task or check flag:</h1>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the dot in the tag's class name is a typo

Comment on lines +20 to +27
background: #91ff7a;
border: none;
border-radius: 4px;
cursor: pointer;
padding: 12px 12px;
font-family: 'Comfortaa', cursive, bold;
font-size: 16px;
font-weight: bold;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation issue, apparently

Comment thread web/cookiesCTF/main.py
Comment on lines +29 to +34
if request.get_cookie("is_admin") == "no":
f = open('./Pages/Task1/Task1GUEST.html')
return f.read()
elif request.get_cookie("is_admin") == "yes":
f = open('./Pages/Task1/Task1ADMIN.html')
return f.read()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And what happens if the cookie is neither yes nor no? The server just doesn't send a response? Tell me if you have done this on purpose, and that's part of the task, but I assume you always want the server to send at least some response.

There are multiple ways to implement this, the simplest way I see is to return the Task1ADMIN.html page if the cookie is set to "yes", and return the guest page in any other case. By the way, I can see that you're already doing a similar thing in task 2. So why not do it here?

Comment thread web/cookiesCTF/main.py
Comment on lines +70 to +72
response.set_cookie("Jake_is_fun",
"35eae3ea7fe1598f7f8ffa2806229068074bac921b2d235ed02660c6009eba9d40b09e2f5bfa099c8e66235d89d2bd3771941741a02a181aba9dcb027acca465",
path='/')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's strange formatting. I don't mind having very long lines, because it's a CTF task source, not a production code or something, but this one's really strange

Comment thread web/cookiesCTF/main.py
Comment on lines +73 to +74
if request.get_cookie(
"Jake_is_fun") == "7d357b285f1744c3af09312e2c2e3c577fec5d0a091cb26e3089624890a31aad4d78165cb696d796f10eb81a568bcfa165a2e54771ad5bfb8f6451e48d3c1159":

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This piece has strange formatting too, btw. Maybe put these two lines on one?

Comment thread web/cookiesCTF/main.py
Comment on lines +121 to +122

run(host='localhost', port=8080, debug=True)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
run(host='localhost', port=8080, debug=True)
if __name__ == "__main__":
run(host='0.0.0.0', port=8080, debug=False)

Tell me if you don't understand something here and this needs comments

Comment thread web/cookiesCTF/styles.css
Comment on lines +19 to +28
input[type=submit] {
background: #91ff7a;
border: none;
border-radius: 4px;
cursor: pointer;
padding: 12px 12px;
font-family: 'Comfortaa', cursive, bold;
font-size: 16;
font-weight: bold;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation issue here

@kolayne

kolayne commented Aug 23, 2021

Copy link
Copy Markdown
Member

By the way, @Dzambek, should we accept tasks with flags containing apostrophes (')?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants