Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add line number when error happens #75

Closed
Liujingfang1 opened this issue Mar 7, 2019 · 3 comments
Closed

Add line number when error happens #75

Liujingfang1 opened this issue Mar 7, 2019 · 3 comments

Comments

@Liujingfang1
Copy link

Got following error

Error: strconv.Atoi: parsing "": invalid syntax

when applying the syntax.

- op: add
  path: /spec/template/spec/containers/0/args/
  value: --default-issuer-kind=ClusterIssuer 
- op: add
  path: /spec/template/spec/containers/0/args/
  value: --default-issuer-name=letsencrypt

It would be nice to have the line number when the error happens.

@donbowman
Copy link

see below to reproduce. Note the missing - on the end of path.

doc.json:

{"apiVersion": "apps/v1beta1", "kind": "Deployment", "metadata": {"name": "DEP"}, "spec": {"replicas": 1, "selector": {"matchLabels": {"app": "DEP"}}, "template": {"metadata": {"labels": {"app": "DEP"}, "annotations": null}, "spec": {"containers": [{"name": "container", "args": ["--cluster-resource-namespace=$(POD_NAMESPACE)", "--leader-election-namespace=$(POD_NAMESPACE)"]}]}}}}

patch.json:

[{"op": "add", "path": "/spec/template/spec/containers/0/args/", "value": "--default-issuer-kind=ClusterIssuer"}, {"op": "add", "path": "/spec/template/spec/containers/0/args/", "value": "--default-issuer-name=letsencrypt"}]
$ cat /tmp/doc.json | ./json-patch -p /tmp/patch.json 
2019/03/07 17:44:23 error applying patch: strconv.Atoi: parsing "": invalid syntax

@evanphx
Copy link
Owner

evanphx commented Apr 29, 2019

Ironically your reproduction doc is only one line. :D

@evanphx
Copy link
Owner

evanphx commented Jun 10, 2019

I can't extract the line number but that commit will now report more about what failed to help diagnose the issue.

atrakh pushed a commit to launchdarkly/json-patch that referenced this issue Jun 24, 2020
* Support array remove with negative index.

* Allow replacement of null values

* Fix tabs

* Handle null paths and ops without panicking.

* Update stable version in Readme

* fix test panic

Without this change, the following code would panic:
```
package main

import (
        "fmt"

        jsonpatch "github.com/evanphx/json-patch"
)

func main() {
        original := []byte(`{"name": "John", "age": 24, "height": 3.21}`)
        patchJSON := []byte(`[
                {"op": "test", "path": "/name"}
        ]`)

        patch, err := jsonpatch.DecodePatch(patchJSON)
        if err != nil {
		fmt.Printf("decode %v\n", err)
		return
        }

        modified, err := patch.Apply(original)
        if err != nil {
		fmt.Printf("aplly %v\n", err)
		return
        }

        fmt.Printf("Original document: %s\n", original)
        fmt.Printf("Modified document: %s\n", modified)
}

```

* Add ability to disable non-standard negative indices support

* Add note about SupportNegativeIndices

* Fix typo in README

* limit how big array can grow

* Document the ArraySizeLimit global variable

* adding limit on extending array size

* Add doc for the ArraySizeAdditionLimit

* Deep copy the source to prevent the copy operation to create loops

* The "copy" operation should "add" the value to the target location,
instead of "set".

* Adding a configurable limit for accumulated size increase caused by the copy
operations in a patch.

* converting ArrarySizeError to a typed one

* Setting and resetting global configurations in the test case.

Previously, the global configurations are set in the init function in
the unit test. This can bite importers' unit tests if they don't prune
vendored tests.

* According to the RFC, the move operation should be functionally
identical to a remove followed by an add.

* Since that the "set" method is now only called to implement the
"replace" operation, and the "path" of "replace" must refer to an
existing location, we can greatly simplify the sanity check in "set".

Also, we removed the ArraySizeAdditionLimit, because according to the RFC, no
operation can increase the length of an existing array by more than 1.

* Removed the ArraySizeLimit.

It's not useful anymore because:
1. There used to be bugs in the library that allowed the "path" field of
the "move" and "copy" operations to refer to arbitrarily large index of
an existing array. Now the bugs are fixed, the two operations can at
most increase the length of any _existing_ array by 1, by appending to the
end.

2. The "add" and "replace" operation can create large new arrays.
However, for these two operations, the patch MUST contain the "value",
thus the caller of this library can cap the size by limiting the size of the
patch.

3. The "copy" operations can copy existing large array, but users can
control the size increase by setting AccumulatedCopySizeLimit.

* Expose Operation and methods, fix lint errors

* Update error messages

* Revert govet fixes

* Cleanup errors to help understand what failed. Fixes evanphx#75

* Add bits to better be a go module

* check lengths of maps and recurse over only one if it is necessary

* add benchmark tests for matchesValue

* Fix panic when SupportNegativeIndices is false

If SupportNegativeIndices is false then a negative indicie will cause a panic.
This regression was introduced in: fcd53ec
Before that change we always checked the validity of the negative index.

* Conform to RFC6902 replacement semantics.

A "replace" patch operation referencing a key that does not exist in the
target object are currently being accepted; ultimately becoming "add"
operations on the target object. This is incorrect per the
specification:

From https://tools.ietf.org/html/rfc6902#section-4.3 section about
"replace":

    The target location MUST exist for the operation to be successful.

This corrects the behavior by returning an error in this situation.

* Test for copying non-existent key.

* Test for testing non-existent and null-value keys.

* Test for copying null-value key.

* Equality comparison bug fix

* Fixed output of example

Made the output text in the examples match what the code does

* Typo in README

* Fix Issue evanphx#88, add more tests for the Equals() function.

* added some more Equals() tests

* fix little typo in README too

* copy go sources into v5

* update paths for v5

* correct module requirements

* also test v5 in travis

* update travis to most recent go versions

* Update instructions to use v5 dir.

* Resolve a couple of differences
t

* Return to panic

* allow replacing nil values

* update imports

* update gomod

* update gomod again

Co-authored-by: Evan Phoenix <evan@fallingsnow.net>
Co-authored-by: Cao Shufeng <caosf.fnst@cn.fujitsu.com>
Co-authored-by: Evan Phoenix <evan@phx.io>
Co-authored-by: Carlos Cobo <toqueteos@gmail.com>
Co-authored-by: Chao Xu <xuchao@google.com>
Co-authored-by: Sam Ward <github@ward.io>
Co-authored-by: Han Kang <hankang@google.com>
Co-authored-by: Eric Paris <eparis@redhat.com>
Co-authored-by: Brett Buddin <brett@buddin.org>
Co-authored-by: Igor Gubernat <igorgubernat@gmail.com>
Co-authored-by: Bård Aase <elzapp@gmail.com>
Co-authored-by: Arnaud Rebillout <arnaud.rebillout@collabora.com>
Co-authored-by: jackhafner <jackhaf@gmail.com>
Co-authored-by: Benjamin Elder <ben@elder.dev>
Co-authored-by: atrakh <arnold@launchdarkly.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants