Skip to content

Commit

Permalink
MB-6682 create and use new 404 page design (#9226)
Browse files Browse the repository at this point in the history
* Redesign the 404 page and update the tests

* Add route to not found to the office app

* Add in middleware routing that directly disables the static and downloads paths, but not the subpaths

* Change the SPA handler rather than routing_init

* Return an error instead of the static directory

* Revert "Return an error instead of the static directory"

This reverts commit a5b27fb.

* Add in font family so that the styles look the same between the office app and the customer app

* Add test for 404 page on the office side

* Add a custom fileSystem and use that inside of routing_init instead of the spaHandler

* Fix routing so to use the build root

- include the build root when building the custom file system
- add more logging and error messages in the cutom file system

* Use path.join instead of appending stuff together

* Refactor the spa_handler to use the custom_filesystem

- delete the custom_filesystem file and just directly have it inside of the spa_handler
- use the custom_filesystem inside of the spa_handler to handle directory listing

* Expose the custom filesystem so that it can be mocked in the tests

* Add initial suite of tests

* Fix test cases

- remove the if check from the test since it should be ok without
- fix test case so that it actually tests what is being said
- add missing slashes

* Fix tests for spa handler

- use both afero's MemMapFs and OsFs to create the tests files
- add a cleanup function

* Add debugging statements to see what's going on

* Remove the os check and adjust tests

* Remove all the debug statements and unused code

- take out the os check inside of ServeHTTP since it is already being covered by the custom
file system's open

* Adjust font size and spacing for the not found pages

* Remove the prose designation

* Fix spacing issues

- adjust all the spacing between text, header, and footer

* Remove the font famiy since it was creating unexpected behavior

* Fix additional spacing discrepancies and how the go back button works

- change the go back button to navigate to the queue instead of going back

* Fix font size for mobile

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
YanZ777 and mergify[bot] committed Nov 4, 2022
1 parent b46b8e1 commit 63f86c7
Show file tree
Hide file tree
Showing 10 changed files with 343 additions and 37 deletions.
7 changes: 7 additions & 0 deletions pkg/handlers/routing/routing_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,16 @@ func InitRouting(appCtx appcontext.AppContext, redisPool *redis.Pool,
clientCertMiddleware := authentication.ClientCertMiddleware(appCtx)

// Serves files out of build folder
cfs := handlers.NewCustomFileSystem(
http.Dir(routingConfig.BuildRoot),
"index.html",
appCtx.Logger(),
)

clientHandler := handlers.NewSpaHandler(
routingConfig.BuildRoot,
"index.html",
cfs,
)

// Stub health check
Expand Down
72 changes: 52 additions & 20 deletions pkg/handlers/spa_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ package handlers

import (
"net/http"
"os"
"path/filepath"

"go.uber.org/zap"

"github.com/transcom/mymove/pkg/logging"
)

Expand All @@ -17,14 +18,59 @@ import (
type SpaHandler struct {
staticPath string
indexPath string
cfs CustomFileSystem
}

// NewSpaHandler returns a new handler for a Single Page App
func NewSpaHandler(staticPath string, indexPath string) SpaHandler {
func NewSpaHandler(staticPath string, indexPath string, cfs CustomFileSystem) SpaHandler {
return SpaHandler{
staticPath: staticPath,
indexPath: indexPath,
cfs: cfs,
}
}

// from https://www.alexedwards.net/blog/disable-http-fileserver-directory-listings
type CustomFileSystem struct {
fs http.FileSystem
indexPath string
logger *zap.Logger
}

func NewCustomFileSystem(fs http.FileSystem, indexPath string, logger *zap.Logger) CustomFileSystem {
return CustomFileSystem{
fs: fs,
indexPath: indexPath,
logger: logger,
}
}

func (cfs CustomFileSystem) Open(path string) (http.File, error) {
f, openErr := cfs.fs.Open(path)
logger := cfs.logger
logger.Debug("Using CustomFileSystem for " + path)

if openErr != nil {
logger.Error("Error with opening", zap.Error(openErr))
return nil, openErr
}

s, _ := f.Stat()
if s.IsDir() {
index := filepath.Join(path, cfs.indexPath)
if _, indexOpenErr := cfs.fs.Open(index); indexOpenErr != nil {
closeErr := f.Close()
if closeErr != nil {
logger.Error("Unable to close ", zap.Error(closeErr))
return nil, closeErr
}

logger.Error("Unable to open index.html in the directory", zap.Error(indexOpenErr))
return nil, indexOpenErr
}
}

return f, nil
}

// ServeHTTP inspects the URL path to locate a file within the static dir
Expand All @@ -34,33 +80,19 @@ func NewSpaHandler(staticPath string, indexPath string) SpaHandler {
func (h SpaHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
logger := logging.FromContext(r.Context())
logger.Debug("Using SPA Handler for " + r.URL.Path)

// get the absolute path to prevent directory traversal
path, err := filepath.Abs(r.URL.Path)
_, err := filepath.Abs(r.URL.Path)
if err != nil {
// if we failed to get the absolute path respond with a 400 bad request
// and stop
http.Error(w, err.Error(), http.StatusBadRequest)
return
}

// prepend the path with the path to the static directory
path = filepath.Join(h.staticPath, path)

// check whether a file exists at the given path
_, err = os.Stat(path)
if os.IsNotExist(err) {
// file does not exist, serve index.html
http.ServeFile(w, r, filepath.Join(h.staticPath, h.indexPath))
return
} else if err != nil {
// if we got an error (that wasn't that the file doesn't exist) stating the
// file, return a 500 internal server error and stop
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}

// otherwise, use http.FileServer to serve the static dir
http.FileServer(http.Dir(h.staticPath)).ServeHTTP(w, r)
// use the customFileSystem so that we do not expose directory listings
http.FileServer(h.cfs).ServeHTTP(w, r)
}

// NewFileHandler serves up a single file
Expand Down
155 changes: 155 additions & 0 deletions pkg/handlers/spa_handler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
package handlers

import (
"log"
"net/http"
"net/http/httptest"
"testing"

"github.com/spf13/afero"
"github.com/stretchr/testify/suite"
"go.uber.org/zap"

"github.com/transcom/mymove/pkg/testingsuite"
)

type SpaHandlerSuite struct {
*testingsuite.PopTestSuite
logger *zap.Logger
mfs afero.HttpFs
}

func setupMockFileSystem() *afero.HttpFs {
afs := afero.NewMemMapFs()

errMkdir := afs.MkdirAll("test", 0755)

if errMkdir != nil {
log.Panic(errMkdir)
}

errWriteFile := afero.WriteFile(afs, "/test/a", []byte("file a"), 0644)

if errWriteFile != nil {
log.Panic(errWriteFile)
}

errWriteFile = afero.WriteFile(afs, "/test/index.html", []byte("index html file"), 0644)

if errWriteFile != nil {
log.Panic(errWriteFile)
}

errMkdir = afs.MkdirAll("/test/noIndexDir", 0755)

if errMkdir != nil {
log.Panic(errMkdir)
}

errWriteFile = afero.WriteFile(afs, "/test/noIndexDir/b", []byte("file b"), 0644)

if errWriteFile != nil {
log.Panic(errWriteFile)
}

ahttpFs := afero.NewHttpFs(afs)
return ahttpFs
}

func TestSpaHandlerSuite(t *testing.T) {
logger, err := zap.NewDevelopment()
if err != nil {
log.Panic(err)
}

mfs := setupMockFileSystem()

hs := &SpaHandlerSuite{
PopTestSuite: testingsuite.NewPopTestSuite(testingsuite.CurrentPackage(), testingsuite.WithPerTestTransaction()),
logger: logger,
mfs: *mfs,
}
suite.Run(t, hs)
hs.PopTestSuite.TearDown()
}

type testCase struct {
name string
request string
expectedStatusCode int
expectedBody string
}

func (suite *SpaHandlerSuite) TestSpaHandlerServeHttp() {
cases := []testCase{
{
name: "A directory without a trailing slash and that has an index.html",
request: "test",
expectedStatusCode: http.StatusMovedPermanently,
expectedBody: "",
},
{
name: "A directory with a trailing slash and that has an index.html",
request: "test/",
expectedStatusCode: http.StatusOK,
expectedBody: "index html file",
},
{
name: "A directory without a trailing slash and that does not have an index.html",
request: "test/noIndexDir",
expectedStatusCode: http.StatusNotFound,
expectedBody: "404 page not found\n",
},
{
name: "A directory with a trailing slash and that does not have an index.html",
request: "test/noIndexDir/",
expectedStatusCode: http.StatusNotFound,
expectedBody: "404 page not found\n",
},
{
name: "A file that exists in a directory that does have an index.html",
request: "test/a",
expectedStatusCode: http.StatusOK,
expectedBody: "file a",
},
{
name: "A file that exists in a directory that does not have an index.html",
request: "test/noIndexDir/b",
expectedStatusCode: http.StatusOK,
expectedBody: "file b",
},
{
name: "A file that does not exist",
request: "test/c",
expectedStatusCode: http.StatusNotFound,
expectedBody: "404 page not found\n",
},
}

cfs := NewCustomFileSystem(
suite.mfs,
"index.html",
suite.logger,
)

for _, testCase := range cases {
suite.T().Run(testCase.name, func(t *testing.T) {

req, err := http.NewRequest("GET", testCase.request, nil)
suite.NoError(err)

// We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response.
rr := httptest.NewRecorder()

// Our handlers satisfy http.Handler, so we can call their ServeHTTP method
// directly and pass in our Request and ResponseRecorder.
sh := NewSpaHandler("", "index.html", cfs)
sh.ServeHTTP(rr, req)

suite.Equal(testCase.expectedStatusCode, rr.Code, "Status codes did not match when retreiving %v for request %v: expected %v, got %v", testCase.name, testCase.request, testCase.expectedStatusCode, rr.Code)

// Check the response body is what we expect.
suite.Equal(testCase.expectedBody, rr.Body.String(), "Handler returned unexpected body when retrieving %v: expected %v, got %v", testCase.name, testCase.expectedBody, rr.Body.String())
})
}
}
33 changes: 26 additions & 7 deletions src/components/NotFound/NotFound.jsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,34 @@
import React from 'react';
import classnames from 'classnames';
import PropTypes from 'prop-types';
import { Button } from '@trussworks/react-uswds';

import styles from './NotFound.module.scss';

const NotFound = ({ handleOnClick }) => {
return (
<div className="usa-grid">
<div className="grid-container usa-prose">
<h1>Page not found</h1>
<p>Looks like you&apos;ve followed a broken link or entered a URL that doesn&apos;t exist on this site.</p>
<button type="button" className="usa-button" onClick={handleOnClick}>
Go Back
</button>
<div className={classnames('usa-grid', styles.notFound)}>
<div className="grid-container">
<div className={styles.preheader}>
<b>Error - 404</b>
<div className={styles.preheaderQuip}>
<b>Let&apos;s move you in the right direction</b>
</div>
</div>
<h1>
<b>We can&apos;t find the page you&apos;re looking for</b>
</h1>
<div className={styles.description}>
<p className={styles.explanation}>
You are seeing this because the page you&apos;re looking for doesn&apos;t exist or has been removed.
</p>
<p className={styles.recommendation}>
We suggest checking the spelling in the URL or return{' '}
<Button unstyled className={styles.goBack} onClick={handleOnClick}>
back home.
</Button>
</p>
</div>
</div>
</div>
);
Expand Down

0 comments on commit 63f86c7

Please sign in to comment.