xkcd comics data provider

Please help me improving this code(golang), thanks!

Why I need built a API for xkcd

-
Can't get the data provided by xkcd directly in front-end(CORS).

-
So we can package the data to 10 comics per request. Instead of 1 request match one comic.

main.go

``
package main

import (
"encoding/json"
"fmt"
"log"
"net/http"
"strconv"
"strings"
)

type comicInfo struct {
Number int json:"num"
Img string json:"img"
Title string json:"title"
Alt string json:"alt"`
}

var comicInfoMap = make(map[int]comicInfo)
var newestComic comicInfo

func fetch(url string, ch chan<- comicInfo) {
if res, err := http.Get(url); err == nil {
if res.StatusCode == http.StatusOK {
newComic := comicInfo{}
if err := json.NewDecoder(res.Body).Decode(&newComic); err == nil {
fmt.Println(newComic)
ch <- newComic
}
}
}
}

func init() {
ch := make(chan comicInfo)
if res, err := http.Get("http://xkcd.com/info.0.json"); err == nil {
if res.StatusCode == http.StatusOK {
newComic := comicInfo{}
if err := json.NewDecoder(res.Body).Decode(&newComic); err == nil {
newestComic = newComic
comicInfoMap[newestComic.Number] = newestComic
for j := 0; j < 10; j++ {
addtion := 100 * j
for i := addtion + 1; i < addtion+101; i++ {
indexStr := strconv.Itoa(newComic.Number - i)
url := "http://xkcd.com/" + indexStr + "/info.0.json"
go fetch(url, ch)
}
for i := addtion + 1; i < addtion+101; i++ {
newComic := <-ch
comicInfoMap[newComic.Number] = newComic
}
}
}
}
}
}

func main() {
http.HandleFunc("/", handler)
log.

Being inspired by recent xkcd comic I will review this particular piece of code:

res := strings.Split(r.URL.Path, "/")
path, err := strconv.Atoi(res[1])

As far as I understand it converts path string "/123" to int 123.

I see several issues here:

• this code creates unnecessary (and possibly large) slice,

• assumes that res[1] is always there,

• parses negative numbers,

• and allows input /123/whatever/goes/here/.

Split

Use strings.SplitN when you know how many parts of the slice you need. On /a/b/c/d/ input strings.Split will create slice with six items, but you need only res[1].

..

url.URL documentation states, that relative paths may omit leading slash. So on .. or . input access to res[1] will trigger panic with "runtime error: index out of range" message.

Malicious HTTP request may contain lots of crap similar ../.. so is crucial to validate this.

I suggest you to check that the path begins with a slash or test the length of res.

stronv.ParseUint

strconv.Atoi allows negative numbers. You can use strconv.ParseUint instead.

Path from HTTP request is same thing as any other form of user input. Proper handling of it is full of gotchas.

I suggest you to check that path begins with a slash and use strings.TrimSuffix to remove one possible trailing slash:

// Check that r.URL.Path begins with "/",
if (r.URL.Path[0] == "/") {
    // skip leading slash and trim one(!) possible trailing shash.
    num := strings.TrimSuffix(r.URL.Path[1:], "/")
    // Try to parse non negative number,
    tmp, err := strconv.ParseUint(num, 10, 32)
    if err != nil {
        // and return 404 if there is no one.
    }
    // Convert uint to int.
    index := int(tmp)

    // function body continues...
}

StackExchange Code Review Q#133892, answer score: 3