URL Validation Function Needs Fixes

[dhextras]

Validate URL Issues:

1. Short URL Check:

   • The function does not account for the https:// prefix when checking the length of the URL, which means it would never reach min length validation checks which is 4.

2. TLD Checking:

   • The TLD validation is inadequate; it simply checks for a . in the URL, allowing invalid URLs like https://lol/some.thing to pass.

3. Domain Extraction:

   • The function incorrectly considers the path of the URL as part of the domain. Only the hostname should be extracted and validated separately.

4. Lack of URL Sanitization:

   • There’s no sanitization of the URL, meaning special characters are not handled correctly, like ][\@! etc...

Validate Code Issues:

• Errors returned are not used by the consumers of the function nor presented to users, making it difficult to identify issue for the users and rendering the returned argumenets usless.

[Hamza12700]

Errors returned are not used by the consumers of the function nor presented to users, making it difficult to identify issue for the users and rendering the returned argumenets usless.

That is because the error is only for debugging and logging purposes, showing the error of the code for the short link is invalid seems kind a wired in a normal user's perspective. So that is why I decided to not show but instead redirect the user to home page.

But if you've a good reasons about why we should send the error message for the invalid code then I'd like to hear that.

[dhextras]

Errors returned are not used by the consumers of the function nor presented to users, making it difficult to identify issue for the users and rendering the returned argumenets usless.

No, that's wrong, errors returned by the ValidateUrl function call is been sent to the user: shawty/handlers/shawty.go

func Shawty(w http.ResponseWriter, r *http.Request) {
        // code...

  // Validate the URL
  err := validate.ValidateUrl(inputUrl)
  if err != nil {
      Logger.Warn("failed to validate URL", "url", inputUrl, "user-agent", r.UserAgent(), "error", err)
      errorTempl := template.Must(template.ParseFiles("./partial-html/short-link-error.html"))
      asserts.NoErr(errorTempl.Execute(w, err), "Failed to execute template short-link-error.html")
      return
  }
}

No I wan't mentioning ValidateUrl but rather ValidateCode that had the issue

func Redirect(w http.ResponseWriter, r *http.Request) {
    code := r.PathValue("code")

    // Validate the code
    err := validate.CustomCodeValidate(code)
    if err != nil {
        Logger.Warn("Code validation failed", "code", code, "user-agent", r.UserAgent())
        http.Redirect(w, r, "/", http.StatusBadRequest)
        return
    }

[Hamza12700]

Sorry, @dhextras I miss read it.

The reason I decide to not send the error message to the user is because the error is only for debugging and logging purposes, showing the error of the code for the short link is invalid seems kind a wired in a user's perspective. So that is why I decided to not show but instead redirect the user to home page.

But if you've a good reasons about why we should send the error message for the invalid code then I'd like to hear that.

[dhextras]

Sorry, @dhextras I miss read it.

The reason I decide to not send the error message to the user is because the error is only for debugging and logging purposes, showing the error of the code for the short link is invalid seems kind a wired in a normal user's perspective. So that is why I decided to not show but instead redirect the user to home page.

But if you've a good reasons about why we should send the error message for the invalid code then I'd like to hear that.

Im not saying we should show the error to the user when redirecting, at least log them out in the server and instead of showing the plain bad request page we should either redirect them to home page / or show the URL is not valid something like that

[Hamza12700]

Im not saying we should show the error to the user when redirecting, at least log them out in the server and instead of showing the plain bad request page we should either redirect them to home page / or show the URL is not valid something like that

Yes, the errors are being logged when the code is invalid: shawty/handlers/redirect.go

func Redirect(w http.ResponseWriter, r *http.Request) {
    code := r.PathValue("code")

    // Validate the code
    err := validate.CustomCodeValidate(code)
    if err != nil {
        Logger.Warn("Code validation failed", "code", code, "user-agent", r.UserAgent())
        http.Redirect(w, r, "/", http.StatusBadRequest)
        return
    }
}

... and instead of showing the plain bad request page we should either redirect them to home page / or show the URL is not valid something like that

Not sure what you mean by plain old request page but the other part is correct that if the code is invalid or doesn't exist then it redirects the user to the home page /

[dhextras]

Not sure what you mean by plain old request page but the other part is correct that if the code is invalid or doesn't exist then it redirects the user to the home page /

I'm saying it doesn't redirect to the home page but rather just go here, which have Anchor pointed to home page

2. you are not actually logging the err it self, just logging that the code validation failed and not why it failed

[Hamza12700]

you are not actually logging the err it self, just the code validation failed and not why it failed

What you mean not logging the error itself? The value that is being returned by the validate.CustomCodeValidate function is the error why the code validation failed. What other error are you referring to?

[Hamza12700]

This is because of the status code. It should be 303 not 400

[dhextras]

@Hamza12700

When the validation fails its logs Code validation failed which is not the err that is being returned from the function, which is one of the below error from here

[dhextras]

This is because of the status code. It should be 303 not 400

i didn't get that why would it return 400 if it's expecting to return 303. can you explain more about this ?

[Hamza12700]

i didn't get that why would it return 400 if it's expecting to return 303. can you explain more about this ?

The server returns 400 a bad request because the code for the short link is either invalid or doesn't exist. It should return 303 in order to redirect the user to home page.

[Hamza12700]

@Hamza12700

When the validation fails its logs Code validation failed which is not the err that is being returned from the function, which is one of the below error from here

I must of forgot to add log the error too.

[dhextras]

@Hamza12700 When the validation fails its logs Code validation failed which is not the err that is being returned from the function, which is one of the below error from here

I must of forgot to add log the error too.

yeah that's what i was refering to

[dhextras]

i didn't get that why would it return 400 if it's expecting to return 303. can you explain more about this ?

The server returns 400 a bad request because the code for the short link is either invalid or doesn't exist. It should return 303 in order to redirect the user to home page.

Okay I still don't understand if it's not gonna redirect to the home page y even use StatusBadRequest rather then something that will return in 3xx

http.Redirect(w, r, "/", http.StatusBadRequest)

[dhextras]

i didn't get that why would it return 400 if it's expecting to return 303. can you explain more about this ?

The server returns 400 a bad request because the code for the short link is either invalid or doesn't exist. It should return 303 in order to redirect the user to home page.

Okay I still don't understand if it's not gonna redirect to the home page y even use StatusBadRequest rather then something that will return in 3xx

http.Redirect(w, r, "/", http.StatusBadRequest)

what's the point of this redirection? is it to redirect users to the home page?

[Hamza12700]

what's the point of this redirection? is it to redirect users to the home page?

Yes, if the code is invalid or doesn't exist redirect them to the home page

[Hamza12700]

Okay I still don't understand if it's not gonna redirect to the home page y even use StatusBadRequest rather then something that will return in 3xx

My bad, I thought because it's taking user input then if the code is invalid or doesn't exists then it should be a status bad request but doing the browser doesn't redirect the user.

[dhextras]

Yes, if the code is invalid or doesn't exist redirect them to the home page

Then y use the status code 400 -- StatusBadRequest which clearly doesn't redirect

[dhextras]

Okay I still don't understand if it's not gonna redirect to the home page y even use StatusBadRequest rather then something that will return in 3xx

My bad, I thought because it's taking user input then if the code is invalid or doesn't exists then it should be a status bad request but doing the browser doesn't redirect the user.

Okay that's what I was asking, cool if you fixed it, what about the rest of the issue ( the validate URL part )

[Hamza12700]

Then y use the status code 400 -- StatusBadRequest which clearly doesn't redirect

It was a mistake man. I forgot that sending a status bad request doesn't work if trying to redirect.

[dhextras]

Then y use the status code 400 -- StatusBadRequest which clearly doesn't redirect

It was a mistake man. I forgot that sending a status bad request doesn't if trying to redirect.

LOL fine, since you were arguing I was confused haha

[Hamza12700]

Okay that's what I was asking, cool if you fixed it, what about the rest of the issue ( the validate URL part )

The rest are all valid issues. They need to be fixed.

[dhextras]

Okay that's what I was asking, cool if you fixed it, what about the rest of the issue ( the validate URL part )

The rest are all valid issues. They need to fixed.

Alr ill work on it

[dhextras]

Then y use the status code 400 -- StatusBadRequest which clearly doesn't redirect

It was a mistake man. I forgot that sending a status bad request doesn't if trying to redirect.

LOL fine, since you were arguing I was confused and thought there was a reason for it haha

[Hamza12700]

LOL fine, since you were arguing I was confused haha

What? I wasn't arguing I was just trying to explain to you what is the issue you're are having.

[dhextras]

LOL fine, since you were arguing I was confused haha

What? I wasn't arguing I was just trying to explain to you what is the issue you are having.

lol fine man, I was just confused, and thought there might be a reason for it, Appreciate the explanation