Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MaxJSONSize and MaxXMLSize must be greater than zero #114

Open
wants to merge 250 commits into
base: main
Choose a base branch
from

Conversation

mstgnz
Copy link

@mstgnz mstgnz commented Dec 24, 2023

Hello,

I have thoroughly enjoyed watching almost all of your courses on Udemy. First and foremost, thank you very, very much for these valuable lessons.

I was following your Tools package course, and a question arose in my mind regarding Json. Therefore, I forked the Toolbox project and tested the issue that puzzled me.

It seems that if MaxJSONSize and MaxXMLSize values are not equal to zero, they are being paired with defaultMaxUpload. In the tests, since these values are always positive numbers, the tests were successful. However, when a negative value was entered, it was not equal to zero, so the entered negative value was assigned, causing the tests to fail.

Therefore, instead of checking whether MaxJSONSize and MaxXMLSize values are equal to zero, I checked whether they are greater than zero. This way, even if a negative value is entered, the tests are successful. Of course, alternatively, it could be considered to return an error by checking whether MaxJSONSize and MaxXMLSize values are less than zero. You know better which approach is more effective.

Since there is no explanation about contributing, I hesitated about the pull request. Nevertheless, I thought I would try my luck.

Update tests for DownloadStaticFile
improve tests & allow for custom max file upload size.
@mstgnz
Copy link
Author

mstgnz commented Dec 28, 2023

Hi @tsawler

I integrated the InArray and LoadSQLQueries methods into the tools package.

InArray can check if the searched key exists in all arrays, regardless of their type.

LoadSQLQueries allows us to use SQL codes in SQL files instead of using them in code, thus avoiding code pollution.

I thought it would be good to have them in a toolbox.

@tsawler
Copy link
Owner

tsawler commented Dec 29, 2023

Thanks for this. I'll give it a look as soon as I can.

@@ -77,7 +77,7 @@ func (t *Tools) ReadJSON(w http.ResponseWriter, r *http.Request, data interface{
maxBytes := defaultMaxUpload

// If MaxJSONSize is set, use that value instead of default.
if t.MaxJSONSize != 0 {
if t.MaxJSONSize > 0 {
Copy link
Author

Choose a reason for hiding this comment

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

Therefore, instead of checking whether MaxJSONSize and MaxXMLSize values are equal to zero, I checked whether they are greater than zero. This way, even if a negative value is entered, the tests are successful. Of course, alternatively, it could be considered to return an error by checking whether MaxJSONSize and MaxXMLSize values are less than zero. You know better which approach is more effective.

Copy link
Author

@mstgnz mstgnz left a comment

Choose a reason for hiding this comment

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

I didn't write a test for whether the sql file is valid or not. You can write a test for this, but I don't know if it is necessary.

load_sql.go Show resolved Hide resolved
load_sql.go Show resolved Hide resolved
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.

None yet

2 participants