add sanity checks for base router

g-harel · Jun 6, 2019 · f352c0b · f352c0b
1 parent b527c04
commit f352c0b
Show file tree

Hide file tree

Showing 12 changed files with 539 additions and 336 deletions.
diff --git a/handlers/compare.go b/handlers/compare.go
@@ -17,84 +17,86 @@ import (
 )
 
 // Compare handler displays a diff between two package versions.
-func Compare(w http.ResponseWriter, r *http.Request) {
-	vars := mux.Vars(r)
-	name := vars["name"]
-	versionA := vars["a"]
-	versionB := vars["b"]
+func Compare(ry registry.Registry) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		vars := mux.Vars(r)
+		name := vars["name"]
+		versionA := vars["a"]
+		versionB := vars["b"]
 
-	if versionA == versionB {
-		http.NotFound(w, r)
-		return
-	}
+		if versionA == versionB {
+			http.NotFound(w, r)
+			return
+		}
 
-	// Download both package version contents to a temporary directory in parallel.
-	type downloadedDir struct {
-		version string
-		dir     string
-		err     error
-	}
-	dirChan := make(chan downloadedDir)
-	for _, version := range []string{versionA, versionB} {
-		go func(v string) {
-			// Create temporary working directory.
-			dir, err := ioutil.TempDir("", "")
-			if err != nil {
-				dirChan <- downloadedDir{v, "", fmt.Errorf("create temp dir: %v", err)}
-				return
-			}
+		// Download both package version contents to a temporary directory in parallel.
+		type downloadedDir struct {
+			version string
+			dir     string
+			err     error
+		}
+		dirChan := make(chan downloadedDir)
+		for _, version := range []string{versionA, versionB} {
+			go func(v string) {
+				// Create temporary working directory.
+				dir, err := ioutil.TempDir("", "")
+				if err != nil {
+					dirChan <- downloadedDir{v, "", fmt.Errorf("create temp dir: %v", err)}
+					return
+				}
+
+				// Fetch package contents for given version.
+				pkg, err := ry.PackageContents(name, v)
+				if err != nil {
+					// Error not wrapped so it can be checked against "registry.ErrNotFound".
+					dirChan <- downloadedDir{v, "", err}
+					return
+				}
+				defer pkg.Close()
+
+				// Write package contents to directory.
+				err = tarball.Extract(pkg, tarball.Downloader(func(name string) string {
+					return path.Join(dir, strings.TrimPrefix(name, "package"))
+				}))
+				if err != nil {
+					dirChan <- downloadedDir{v, "", fmt.Errorf("download contents: %v", err)}
+					return
+				}
+
+				dirChan <- downloadedDir{v, dir, nil}
+			}(version)
+		}
 
-			// Fetch package contents for given version.
-			pkg, err := registry.NPM.PackageContents(name, v)
-			if err != nil {
-				// Error not wrapped so it can be checked against "registry.ErrNotFound".
-				dirChan <- downloadedDir{v, "", err}
+		// Wait for both version's contents to be downloaded.
+		dirs := map[string]string{}
+		for i := 0; i < 2; i++ {
+			dir := <-dirChan
+			if dir.err == registry.ErrNotFound {
+				http.NotFound(w, r)
 				return
 			}
-			defer pkg.Close()
-
-			// Write package contents to directory.
-			err = tarball.Extract(pkg, tarball.Downloader(func(name string) string {
-				return path.Join(dir, strings.TrimPrefix(name, "package"))
-			}))
-			if err != nil {
-				dirChan <- downloadedDir{v, "", fmt.Errorf("download contents: %v", err)}
+			if dir.err != nil {
+				http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+				log.Printf("ERROR download package '%v': %v", dir.version, dir.err)
 				return
 			}
-
-			dirChan <- downloadedDir{v, dir, nil}
-		}(version)
-	}
-
-	// Wait for both version's contents to be downloaded.
-	dirs := map[string]string{}
-	for i := 0; i < 2; i++ {
-		dir := <-dirChan
-		if dir.err == registry.ErrNotFound {
-			http.NotFound(w, r)
-			return
+			dirs[dir.version] = dir.dir
 		}
-		if dir.err != nil {
+
+		// Compare contents.
+		patches, err := diff.Compare(dirs[versionA], dirs[versionB])
+		if err != nil {
 			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
-			log.Printf("ERROR download package '%v': %v", dir.version, dir.err)
+			log.Printf("ERROR compare package contents: %v", err)
 			return
 		}
-		dirs[dir.version] = dir.dir
-	}
 
-	// Compare contents.
-	patches, err := diff.Compare(dirs[versionA], dirs[versionB])
-	if err != nil {
-		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
-		log.Printf("ERROR compare package contents: %v", err)
-		return
-	}
+		// Cleanup created directories.
+		for _, path := range dirs {
+			_ = os.RemoveAll(path)
+		}
 
-	// Cleanup created directories.
-	for _, path := range dirs {
-		_ = os.RemoveAll(path)
+		// Render page template.
+		templates.PageCompare(name, versionA, versionB, patches).Handler(w, r)
 	}
-
-	// Render page template.
-	templates.PageCompare(name, versionA, versionB, patches).Render(w)
 }
diff --git a/handlers/directory.go b/handlers/directory.go
@@ -15,69 +15,71 @@ import (
 )
 
 // Directory handler displays a directory view of package contents at the provided path.
-func Directory(w http.ResponseWriter, r *http.Request) {
-	vars := mux.Vars(r)
-	name := vars["name"]
-	version := vars["version"]
-	path := vars["path"]
+func Directory(ry registry.Registry) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		vars := mux.Vars(r)
+		name := vars["name"]
+		version := vars["version"]
+		path := vars["path"]
 
-	// Fetch package contents.
-	pkg, err := registry.NPM.PackageContents(name, version)
-	if err == registry.ErrNotFound {
-		http.NotFound(w, r)
-		return
-	}
-	if err != nil {
-		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
-		log.Printf("ERROR fetch package contents: %v", err)
-		return
-	}
-	defer pkg.Close()
+		// Fetch package contents.
+		pkg, err := ry.PackageContents(name, version)
+		if err == registry.ErrNotFound {
+			http.NotFound(w, r)
+			return
+		}
+		if err != nil {
+			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+			log.Printf("ERROR fetch package contents: %v", err)
+			return
+		}
+		defer pkg.Close()
 
-	// Extract files and directories at the given path.
-	dirs := []string{}
-	files := []string{}
-	err = tarball.Extract(pkg, func(name string, contents io.Reader) error {
-		filePath := strings.TrimPrefix(name, "package/")
-		if strings.HasPrefix(filePath, path) {
-			filePath := strings.TrimPrefix(filePath, path)
-			pathParts := strings.Split(filePath, "/")
-			if len(pathParts) == 1 {
-				files = append(files, pathParts[0])
-			} else {
-				dirs = append(dirs, pathParts[0])
+		// Extract files and directories at the given path.
+		dirs := []string{}
+		files := []string{}
+		err = tarball.Extract(pkg, func(name string, contents io.Reader) error {
+			filePath := strings.TrimPrefix(name, "package/")
+			if strings.HasPrefix(filePath, path) {
+				filePath := strings.TrimPrefix(filePath, path)
+				pathParts := strings.Split(filePath, "/")
+				if len(pathParts) == 1 {
+					files = append(files, pathParts[0])
+				} else {
+					dirs = append(dirs, pathParts[0])
+				}
 			}
+			return nil
+		})
+		if err != nil {
+			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+			log.Printf("ERROR extract files from package contents: %v", err)
+			return
 		}
-		return nil
-	})
-	if err != nil {
-		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
-		log.Printf("ERROR extract files from package contents: %v", err)
-		return
-	}
-	if len(dirs) == 0 && len(files) == 0 {
-		http.NotFound(w, r)
-		return
-	}
-
-	// Sort and de-duplicate input slice.
-	cleanup := func(s []string) []string {
-		m := map[string]interface{}{}
-		for _, item := range s {
-			m[item] = true
+		if len(dirs) == 0 && len(files) == 0 {
+			http.NotFound(w, r)
+			return
 		}
-		out := []string{}
-		for key := range m {
-			out = append(out, key)
+
+		// Sort and de-duplicate input slice.
+		cleanup := func(s []string) []string {
+			m := map[string]interface{}{}
+			for _, item := range s {
+				m[item] = true
+			}
+			out := []string{}
+			for key := range m {
+				out = append(out, key)
+			}
+			sort.Strings(out)
+			return out
 		}
-		sort.Strings(out)
-		return out
-	}
 
-	dirs = cleanup(dirs)
-	files = cleanup(files)
-	parts, links := paths.BreakRelative(path)
+		dirs = cleanup(dirs)
+		files = cleanup(files)
+		parts, links := paths.BreakRelative(path)
 
-	// Render page template.
-	templates.PageDirectory(name, version, parts, links, dirs, files).Render(w)
+		// Render page template.
+		templates.PageDirectory(name, version, parts, links, dirs, files).Handler(w, r)
+	}
 }
diff --git a/handlers/file.go b/handlers/file.go
@@ -15,51 +15,53 @@ import (
 )
 
 // File handler displays a file view of package contents at the provided path.
-func File(w http.ResponseWriter, r *http.Request) {
-	vars := mux.Vars(r)
-	name := vars["name"]
-	version := vars["version"]
-	path := vars["path"]
+func File(ry registry.Registry) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		vars := mux.Vars(r)
+		name := vars["name"]
+		version := vars["version"]
+		path := vars["path"]
 
-	// Fetch package contents.
-	pkg, err := registry.NPM.PackageContents(name, version)
-	if err == registry.ErrNotFound {
-		http.NotFound(w, r)
-		return
-	}
-	if err != nil {
-		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
-		log.Printf("ERROR fetch package contents: %v", err)
-		return
-	}
-	defer pkg.Close()
+		// Fetch package contents.
+		pkg, err := ry.PackageContents(name, version)
+		if err == registry.ErrNotFound {
+			http.NotFound(w, r)
+			return
+		}
+		if err != nil {
+			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+			log.Printf("ERROR fetch package contents: %v", err)
+			return
+		}
+		defer pkg.Close()
 
-	// Find file contents to use in response.
-	// Contents must be written to a buffer to be used in a template.
-	var file *bytes.Buffer
-	err = tarball.Extract(pkg, func(name string, contents io.Reader) error {
-		if strings.TrimPrefix(name, "package/") == path {
-			file = new(bytes.Buffer)
-			_, err := file.ReadFrom(contents)
-			if err != nil {
-				log.Printf("ERROR copy contents: %v", err)
+		// Find file contents to use in response.
+		// Contents must be written to a buffer to be used in a template.
+		var file *bytes.Buffer
+		err = tarball.Extract(pkg, func(name string, contents io.Reader) error {
+			if strings.TrimPrefix(name, "package/") == path {
+				file = new(bytes.Buffer)
+				_, err := file.ReadFrom(contents)
+				if err != nil {
+					log.Printf("ERROR copy contents: %v", err)
+				}
 			}
+			return nil
+		})
+		if err != nil {
+			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+			log.Printf("ERROR extract files from package contents: %v", err)
+			return
+		}
+		if file == nil {
+			http.NotFound(w, r)
+			return
 		}
-		return nil
-	})
-	if err != nil {
-		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
-		log.Printf("ERROR extract files from package contents: %v", err)
-		return
-	}
-	if file == nil {
-		http.NotFound(w, r)
-		return
-	}
 
-	parts, links := paths.BreakRelative(path)
-	lines := strings.Split(file.String(), "\n")
+		parts, links := paths.BreakRelative(path)
+		lines := strings.Split(file.String(), "\n")
 
-	// Render page template.
-	templates.PageFile(name, version, parts, links, lines).Render(w)
+		// Render page template.
+		templates.PageFile(name, version, parts, links, lines).Handler(w, r)
+	}
 }