serg2-mirrors/GDBrowser
1
0
Fork 0
Code Issues 8 Pull requests 10 Projects Releases Packages Wiki Activity Actions

A lot of refactoring, and etc #232

Open
Rudxain wants to merge 62 commits from Rudxain/patch-1 into master
pull from: Rudxain/patch-1
merge into: serg2-mirrors:master
Conversation 14 Commits 62 Files changed 61 +2167 -1946
Rudxain commented 2022-05-27 01:13:18 -04:00 (Migrated from github.com)
Copy link

Fixes #246
The title is self-descriptive. I also renamed misleading names of functions, and activated JS strict mode.

I made small style changes, like making 1-liner functions actually use 1 line, and splitting long lines of property access into multiple lines for easier reading

Fixes #246 The title is self-descriptive. I also renamed misleading names of functions, and activated JS strict mode. I made small style changes, like making 1-liner functions actually use 1 line, and splitting long lines of property access into multiple lines for easier reading
Rudxain commented 2022-05-27 01:16:18 -04:00 (Migrated from github.com)
Copy link

Wait, I forgot to check if any renamed function is used outside the file. I'll convert to draft

Wait, I forgot to check if any renamed function is used outside the file. I'll convert to draft
Rudxain commented 2022-05-27 01:33:44 -04:00 (Migrated from github.com)
Copy link

It seems no functionality should break after renaming, AFAIK

It seems no functionality should break after renaming, AFAIK
Rudxain commented 2022-06-12 15:40:27 -04:00 (Migrated from github.com)
Copy link

I need to do more refactoring and solve the commit incompatibility. Converting to draft

I need to do more refactoring and solve the commit incompatibility. Converting to draft
GDColon commented 2022-06-12 17:26:30 -04:00 (Migrated from github.com)
Copy link

(whoops sorry)
new conflicts aside this looks good, i was just reluctant at first since theres things i do and dont like

(whoops sorry) new conflicts aside this looks good, i was just reluctant at first since theres things i do and dont like
👍 1 😆 1
Rudxain commented 2022-06-13 11:00:57 -04:00 (Migrated from github.com)
Copy link

It's okay. I realized I made a mistake in the formatting. For example, I found an else clause formatted like this:

if (...) {
    ...
}

else {
    ...
}

And I thought it was a whitespace typo because it looked weird and unique, but then I found it's actually a pattern found in many places lol

It's okay. I realized I made a mistake in the formatting. For example, I found an `else` clause formatted like this: ```js if (...) { ... } else { ... } ``` And I thought it was a whitespace typo because it looked weird and unique, but then I found it's actually a pattern found in many places lol
GDColon commented 2022-06-13 21:05:55 -04:00 (Migrated from github.com)
Copy link

It's okay. I realized I made a mistake in the formatting. For example, I found an else clause formatted like this:

if (...) {
    ...
}

else {
    ...
}

And I thought it was a whitespace typo because it looked weird and unique, but then I found it's actually a pattern found in many places lol

oh this is always how i've done it, pls dont change

> It's okay. I realized I made a mistake in the formatting. For example, I found an `else` clause formatted like this: > > ```js > if (...) { > ... > } > > else { > ... > } > ``` > > And I thought it was a whitespace typo because it looked weird and unique, but then I found it's actually a pattern found in many places lol oh this is always how i've done it, pls dont change
👍 1
Rudxain commented 2022-06-14 14:19:54 -04:00 (Migrated from github.com)
Copy link

lmao, I was using a VScode extension to edit repos remotely without cloning, and I thought I had to type the branch name everytime I wanted to commit. Sorry for any confusion to everyone. Now the commits are named using unhelpful names

lmao, I was using a VScode extension to edit repos remotely without cloning, and I thought I had to type the branch name everytime I wanted to commit. Sorry for any confusion to everyone. Now the commits are named using unhelpful names
😆 1
Rudxain commented 2022-06-16 02:01:59 -04:00 (Migrated from github.com)
Copy link

I'm almost done with this PR. If I have enough time, it'll be ready in some hours (or a day) and the commit will also fix #246 .

I did some tests, but I didn't do any tests for stuff that I read enough (both GDB code and MDN docs). I also didn't test if activating strict mode throws an error somewhere lol, the code seems hi-quality enough that we don't need to worry about invalid code.

Also, randRange may not be available due to a race condition while loading global.js, but since the script explicitly starts loading in the HTML before randRange is called, it seems the glitch is unlikely to happen. I guess I can test that, because I don't need NodeJS for that (I don't have NodeJS installed yet, lol)

I'm almost done with this PR. If I have enough time, it'll be ready in some hours (or a day) and the commit will also fix #246 . I did some tests, but I didn't do any tests for stuff that I read enough (both GDB code and MDN docs). I also didn't test if activating strict mode throws an error somewhere lol, the code seems hi-quality enough that we don't need to worry about invalid code. Also, `randRange` may not be available due to a race condition while loading `global.js`, but since the script explicitly starts loading in the HTML before `randRange` is called, it seems the glitch is unlikely to happen. I guess I can test that, because I don't need NodeJS for that (I don't have NodeJS installed yet, lol)
github-advanced-security[bot] (Migrated from github.com) reviewed 2022-06-16 20:39:25 -04:00
misc/manual parsers/parseIconPlist.js
github-advanced-security[bot] (Migrated from github.com) commented 2022-06-16 20:39:24 -04:00
Copy link

Incomplete string escaping or encoding

This replaces only the first occurrence of '}}'.

Show more details

## Incomplete string escaping or encoding This replaces only the first occurrence of '}}'. [Show more details](https://github.com/GDColon/GDBrowser/security/code-scanning/69)
github-advanced-security[bot] (Migrated from github.com) commented 2022-06-16 20:39:25 -04:00
Copy link

Incomplete string escaping or encoding

This replaces only the first occurrence of '{{'.

Show more details

## Incomplete string escaping or encoding This replaces only the first occurrence of '{{'. [Show more details](https://github.com/GDColon/GDBrowser/security/code-scanning/70)
Rudxain (Migrated from github.com) reviewed 2022-06-16 21:04:57 -04:00
misc/manual parsers/parseIconPlist.js
Rudxain (Migrated from github.com) commented 2022-06-16 21:04:57 -04:00
Copy link

I realized replaceAll is better there

I realized `replaceAll` is better there
Rudxain commented 2022-06-16 21:12:20 -04:00 (Migrated from github.com)
Copy link

@GDColon It's ready. But I'm afraid there may be a bug or syntax error somewhere lol, sorry

@GDColon It's ready. But I'm afraid there may be a bug or syntax error somewhere lol, sorry
Rudxain commented 2022-06-25 20:13:57 -04:00 (Migrated from github.com)
Copy link

Wait, I realized that the split method has a 2nd arg that can improve speed and memory use (only in engines that don't optimize well).
Also, the XOR class can be refactored to convert all its prototype.* methods into static ones, that will allow you to do this:

XOR.encrypt(str, key)

instead of this:

const xor = new XOR()
xor.encrypt(str, key)

Edit: I found a possible problem, app.xor. It seems like that property must be an object instance of XOR, I don't know why lol

Wait, I realized that the `split` method has a 2nd arg that can **improve speed and memory use** (only in engines that don't optimize well). Also, the XOR class can be refactored to convert all its `prototype.*` methods into `static` ones, that will allow you to do this: ```js XOR.encrypt(str, key) ``` instead of this: ```js const xor = new XOR() xor.encrypt(str, key) ``` Edit: I found a possible problem, `app.xor`. It seems like that property must be an object instance of `XOR`, I don't know why lol
Rudxain commented 2022-06-26 17:47:35 -04:00 (Migrated from github.com)
Copy link

I need help refactoring obj.foo && obj.foo.bar into obj.foo?.bar. I don't know which ones can be replaced and which not, I tried searching but only got a vague idea

I need help refactoring `obj.foo && obj.foo.bar` into `obj.foo?.bar`. I don't know which ones can be replaced and which not, I tried searching but only got a vague idea
Rudxain commented 2022-06-27 16:41:30 -04:00 (Migrated from github.com)
Copy link

@GDColon I found broken <script src="..."> URLs, like "../icon.js" instead of "/iconkit/icon.js", and "../global.js" instead of "/misc/global.js". I'm fixing it now

Wait holup. I realized the deployed website has a different dir tree, so those paths seem to work?

I noticed iconkit.html doesn't run global.js, why is that?

@GDColon I found broken `<script src="...">` URLs, like "../icon.js" instead of "/iconkit/icon.js", and "../global.js" instead of "/misc/global.js". I'm fixing it now Wait holup. I realized the deployed website has a different dir tree, so those paths seem to work? I noticed `iconkit.html` doesn't run `global.js`, why is that?
github-advanced-security[bot] (Migrated from github.com) reviewed 2022-07-03 13:42:41 -04:00
index.js
@ -343,4 +397,4 @@
else newIconCounts[formName]++
}
})
sacredTexts.newIconCounts = newIconCounts
github-advanced-security[bot] (Migrated from github.com) commented 2022-07-03 13:42:41 -04:00
Copy link

Missing rate limiting

This route handler performs a file system access, but is not rate-limited.

Show more details

## Missing rate limiting This route handler performs [a file system access](1), but is not rate-limited. [Show more details](https://github.com/GDColon/GDBrowser/security/code-scanning/71)
index.js
github-advanced-security[bot] (Migrated from github.com) commented 2022-07-03 13:42:41 -04:00
Copy link

Missing rate limiting

This route handler performs a file system access, but is not rate-limited.

Show more details

## Missing rate limiting This route handler performs [a file system access](1), but is not rate-limited. [Show more details](https://github.com/GDColon/GDBrowser/security/code-scanning/72)
github-advanced-security[bot] (Migrated from github.com) reviewed 2022-07-03 13:45:55 -04:00
api/post/postComment.js
@ -1,3 +2,3 @@
const crypto = require('crypto')
function sha1(data) { return crypto.createHash("sha1").update(data, "binary").digest("hex"); }
const sha1 = data => crypto.createHash("sha1").update(data, "binary").digest("hex")
github-advanced-security[bot] (Migrated from github.com) commented 2022-07-03 13:45:55 -04:00
Copy link

Use of a broken or weak cryptographic algorithm

Sensitive data from an access to username is used in a broken or weak cryptographic algorithm.
Sensitive data from an access to userName is used in a broken or weak cryptographic algorithm.

Show more details

## Use of a broken or weak cryptographic algorithm Sensitive data from [an access to username](1) is used in a broken or weak cryptographic algorithm. Sensitive data from [an access to userName](2) is used in a broken or weak cryptographic algorithm. [Show more details](https://github.com/GDColon/GDBrowser/security/code-scanning/73)
api/post/postProfileComment.js
@ -1,3 +2,3 @@
const crypto = require('crypto')
function sha1(data) { return crypto.createHash("sha1").update(data, "binary").digest("hex"); }
const sha1 = data => crypto.createHash("sha1").update(data, "binary").digest("hex")
github-advanced-security[bot] (Migrated from github.com) commented 2022-07-03 13:45:55 -04:00
Copy link

Use of a broken or weak cryptographic algorithm

Sensitive data from an access to username is used in a broken or weak cryptographic algorithm.
Sensitive data from an access to userName is used in a broken or weak cryptographic algorithm.

Show more details

## Use of a broken or weak cryptographic algorithm Sensitive data from [an access to username](1) is used in a broken or weak cryptographic algorithm. Sensitive data from [an access to userName](2) is used in a broken or weak cryptographic algorithm. [Show more details](https://github.com/GDColon/GDBrowser/security/code-scanning/74)
github-advanced-security[bot] (Migrated from github.com) reviewed 2022-07-03 13:52:59 -04:00
html/iconkit.html
github-advanced-security[bot] (Migrated from github.com) commented 2022-07-03 13:52:59 -04:00
Copy link

Inclusion of functionality from an untrusted source

Script loaded from content delivery network with no integrity check.

Show more details

## Inclusion of functionality from an untrusted source Script loaded from content delivery network with no integrity check. [Show more details](https://github.com/GDColon/GDBrowser/security/code-scanning/76)
html/level.html
@ -261,4 +261,3 @@
if (dailyTime) {
$('#dailyTime').html(` (${colonize(dailyTime, true)})`)
setInterval(() => {
github-advanced-security[bot] (Migrated from github.com) commented 2022-07-03 13:52:58 -04:00
Copy link

Potentially unsafe external link

External links without noopener/noreferrer are a potential security risk.

Show more details

## Potentially unsafe external link External links without noopener/noreferrer are a potential security risk. [Show more details](https://github.com/GDColon/GDBrowser/security/code-scanning/75)
Rudxain (Migrated from github.com) reviewed 2022-07-03 14:01:56 -04:00
html/iconkit.html
Rudxain (Migrated from github.com) commented 2022-07-03 14:01:56 -04:00
Copy link

Wait what? I thought that's just the minified version. Is this because I didn't modify package.json to reflect the new dependency?

Wait what? I thought that's just the minified version. Is this because I didn't modify `package.json` to reflect the new dependency?
Rudxain (Migrated from github.com) reviewed 2022-07-03 14:04:20 -04:00
api/post/postProfileComment.js
@ -1,3 +2,3 @@
const crypto = require('crypto')
function sha1(data) { return crypto.createHash("sha1").update(data, "binary").digest("hex"); }
const sha1 = data => crypto.createHash("sha1").update(data, "binary").digest("hex")
Rudxain (Migrated from github.com) commented 2022-07-03 14:04:20 -04:00
Copy link

LMAO I just changed the function to arrow fn

LMAO I just changed the function to arrow fn
Rudxain (Migrated from github.com) reviewed 2022-07-03 14:04:42 -04:00
api/post/postComment.js
@ -1,3 +2,3 @@
const crypto = require('crypto')
function sha1(data) { return crypto.createHash("sha1").update(data, "binary").digest("hex"); }
const sha1 = data => crypto.createHash("sha1").update(data, "binary").digest("hex")
Rudxain (Migrated from github.com) commented 2022-07-03 14:04:42 -04:00
Copy link

Bruh

Bruh
Rudxain (Migrated from github.com) reviewed 2022-07-03 14:09:29 -04:00
html/level.html
@ -261,4 +261,3 @@
if (dailyTime) {
$('#dailyTime').html(` (${colonize(dailyTime, true)})`)
setInterval(() => {
Rudxain (Migrated from github.com) commented 2022-07-03 14:09:29 -04:00
Copy link

Is this because of using "/" instead of "../"?

Is this because of using "/" instead of "../"?
Rudxain commented 2022-07-03 14:10:41 -04:00 (Migrated from github.com)
Copy link

I think this PR is too big now. I'll re-review to see if I made more mistakes, and only fix those mistakes in this PR. Any further changes will be done in a separate PR

I think this PR is too big now. I'll re-review to see if I made more mistakes, and only fix those mistakes in this PR. Any further changes will be done in a separate PR
Rudxain (Migrated from github.com) reviewed 2022-07-03 14:34:33 -04:00
misc/sampleIcons.json
@ -18,4 +18,3 @@
[ "Cyclic", 30, 3, 12, 0 ],
[ "DanZmeN", 104, 34, 12, 1 ],
[ "DanZmeN", 104, 34, 12, 1 ],
[ "envylol", 73, 20, 1, 1],
Rudxain (Migrated from github.com) commented 2022-07-03 14:34:33 -04:00
Copy link

DanZmeN was duped lol

DanZmeN was duped lol
Rudxain commented 2022-07-03 15:03:12 -04:00 (Migrated from github.com)
Copy link

I'm reverting those URLs because I didn't test if they work

I'm reverting those URLs because I didn't test if they work
Rudxain (Migrated from github.com) reviewed 2022-09-01 04:14:27 -04:00
iconkit/extradata/hardcodedUnlocks.json
Rudxain (Migrated from github.com) commented 2022-09-01 04:14:27 -04:00
Copy link

Why did I delete this?

Why did I delete this?
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin Rudxain/patch-1:Rudxain/patch-1
git checkout Rudxain/patch-1

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git checkout master
git merge --no-ff Rudxain/patch-1
git checkout Rudxain/patch-1
git rebase master
git checkout master
git merge --ff-only Rudxain/patch-1
git checkout Rudxain/patch-1
git rebase master
git checkout master
git merge --no-ff Rudxain/patch-1
git checkout master
git merge --squash Rudxain/patch-1
git checkout master
git merge --ff-only Rudxain/patch-1
git checkout master
git merge Rudxain/patch-1
git push origin master
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

Something isn't working

  
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
enhancement

New feature or request

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
question

Further information is requested

  
wontfix

This will not be worked on

No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: serg2-mirrors/GDBrowser#232
No description provided.
Delete branch "Rudxain/patch-1"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?