Skip to content

Commit

Permalink
[Host service + Frontend] - Fix BrowserData marshalling/unmarshalling…
Browse files Browse the repository at this point in the history
… logic (#6750)

* added test for parsing user data with no bookmars

* added test

* added more tests

* implemented UnmarshalJSON

* fixes

* removed logging + bug fix

* linting

* fixes bookmark import issue

* revisions

* hopefully fix

* Revert "hopefully fix"

This reverts commit 1c473f1.

* undo revision which caused failure
  • Loading branch information
Gabriele Oliaro authored Jun 15, 2022
1 parent 46dc7e7 commit 7c15e4f
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ type Bookmarks struct {
Version int `json:"version,omitempty"`
}

// Custom-defined UnmarshalJSON function to handle the empty-string case correctly.
func (bookmarks *Bookmarks) UnmarshalJSON(data []byte) error {
if string(data) == `""` || string(data) == `''` || string(data) == "" {
return nil
}
type tmp Bookmarks
return json.Unmarshal(data, (*tmp)(bookmarks))
}

// UnmarshalBookmarks takes a JSON string containing bookmark data
// and unmarshals it into a Bookmarks struct, returning the struct
// and any errors encountered.
Expand Down
9 changes: 9 additions & 0 deletions backend/services/host-service/mandelbox/init_browser_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ type BrowserData struct {
ExtensionState types.ExtensionState `json:"extension_state,omitempty"`
}

// Custom-defined UnmarshalJSON function to handle the empty-string case correctly.
func (browserdata *BrowserData) UnmarshalJSON(data []byte) error {
if string(data) == `""` || string(data) == `''` || string(data) == "" {
return nil
}
type tmp BrowserData
return json.Unmarshal(data, (*tmp)(browserdata))
}

// UnmarshalBookmarks takes a JSON string containing all the user browser data
// and unmarshals it into a BrowserData struct, returning the struct
// and any errors encountered.
Expand Down
103 changes: 103 additions & 0 deletions backend/services/host-service/mandelbox/init_browser_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,3 +247,106 @@ func TestUserInitialBrowserParseEmpty(t *testing.T) {
t.Fatalf("UnmarshalBrowserData returned %v, expected %v", unmarshalledBrowserData, userInitialBrowserData)
}
}

func TestUserInitialBrowserParseEmptyBookmarks(t *testing.T) {
// Define variables that will be reused
testCookie1 := "{'creation_utc': 13280861983875934, 'host_key': 'test_host_key_1.com'}"
testCookie2 := "{'creation_utc': 4228086198342934, 'host_key': 'test_host_key_2.com'}"

// We will simulate a user with cookies but no bookmarks
cookiesJSON := "[" + testCookie1 + "," + testCookie2 + "]"
extensions := "not_real_extension_id,not_real_second_extension_id"

// 1. Test case where no bookmark variable is defined (the stringified JSON will not have a `bookmarks` variable)
userInitialBrowserData := BrowserData{
CookiesJSON: types.Cookies(cookiesJSON),
Extensions: types.Extensions(extensions),
}

stringifiedBrowserData, err := json.Marshal(userInitialBrowserData)
if err != nil {
t.Fatalf("could not marshal browser data: %v", err)
}

deflatedBrowserData, err := configutils.GzipDeflateString(string(stringifiedBrowserData))
if err != nil {
t.Fatalf("could not deflate browser data: %v", err)
}

inflatedBrowserData, err := configutils.GzipInflateString(deflatedBrowserData)
if err != nil || len(inflatedBrowserData) == 0 {
t.Fatalf("Could not inflate compressed user browser data: %v", err)
}

// Unmarshal user browser data into proper format
unmarshalledBrowserData, err := UnmarshalBrowserData(types.BrowserData(inflatedBrowserData))
if err != nil {
t.Fatalf("Error unmarshalling user browser data: %v", err)
}

if !cmp.Equal(unmarshalledBrowserData, userInitialBrowserData, cmpopts.EquateEmpty()) {
t.Fatalf("UnmarshalBrowserData returned %v, expected %v", unmarshalledBrowserData, userInitialBrowserData)
}

// 2. Test case where bookmarks are nil
userInitialBrowserData = BrowserData{
CookiesJSON: types.Cookies(cookiesJSON),
Bookmarks: nil,
Extensions: types.Extensions(extensions),
}

stringifiedBrowserData, err = json.Marshal(userInitialBrowserData)
if err != nil {
t.Fatalf("could not marshal browser data: %v", err)
}

deflatedBrowserData, err = configutils.GzipDeflateString(string(stringifiedBrowserData))
if err != nil {
t.Fatalf("could not deflate browser data: %v", err)
}

inflatedBrowserData, err = configutils.GzipInflateString(deflatedBrowserData)
if err != nil || len(inflatedBrowserData) == 0 {
t.Fatalf("Could not inflate compressed user browser data: %v", err)
}

// Unmarshal user browser data into proper format
unmarshalledBrowserData, err = UnmarshalBrowserData(types.BrowserData(inflatedBrowserData))
if err != nil {
t.Fatalf("Error unmarshalling user browser data: %v", err)
}

if !cmp.Equal(unmarshalledBrowserData, userInitialBrowserData, cmpopts.EquateEmpty()) {
t.Fatalf("UnmarshalBrowserData returned %v, expected %v", unmarshalledBrowserData, userInitialBrowserData)
}

// 3. Test case where bookmarks are defined as an empty string in JSON
browserDataString := `{"cookiesJSON":"[{'creation_utc': 13280861983875934, 'host_key': 'test_host_key_1.com'},{'creation_utc': 4228086198342934, 'host_key': 'test_host_key_2.com'}]","bookmarks":"", "extensions":"not_real_extension_id,not_real_second_extension_id"}`
expectedBookmarks := configutils.Bookmarks{}

deflatedBrowserData, err = configutils.GzipDeflateString(string(browserDataString))
if err != nil {
t.Fatalf("could not deflate browser data: %v", err)
}

inflatedBrowserData, err = configutils.GzipInflateString(deflatedBrowserData)
if err != nil || len(inflatedBrowserData) == 0 {
t.Fatalf("Could not inflate compressed user browser data: %v", err)
}

// Unmarshal user browser data into proper format
unmarshalledBrowserData, err = UnmarshalBrowserData(types.BrowserData(inflatedBrowserData))
if err != nil {
t.Fatalf("Error unmarshalling user browser data: %v", err)
}

if !cmp.Equal(*unmarshalledBrowserData.Bookmarks, expectedBookmarks, cmpopts.EquateEmpty()) {
t.Fatalf("UnmarshalBrowserData returned Bookmarks: %v, expected Bookmarks: %v", *unmarshalledBrowserData.Bookmarks, expectedBookmarks)
}

userInitialBrowserData.Bookmarks = &expectedBookmarks

if !cmp.Equal(unmarshalledBrowserData, userInitialBrowserData, cmpopts.EquateEmpty()) {
t.Fatalf("UnmarshalBrowserData returned %v, expected %v", unmarshalledBrowserData, userInitialBrowserData)
}
}
2 changes: 1 addition & 1 deletion frontend/client-applications/src/main/utils/crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ const getBookmarksFromFile = (browser: InstalledBrowser): string => {
// Remove checksum if it exists
delete bookmarksJSON.checksum

return JSON.stringify(bookmarksJSON)
return bookmarksJSON
} catch (err) {
console.error("Could not get bookmarks from file. Error:", err)
return ""
Expand Down

0 comments on commit 7c15e4f

Please sign in to comment.