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

RecursiveEmptyCheck is not honoured by codecgen #224

Closed
bboreham opened this issue Dec 12, 2017 · 5 comments
Closed

RecursiveEmptyCheck is not honoured by codecgen #224

bboreham opened this issue Dec 12, 2017 · 5 comments

Comments

@bboreham
Copy link

I noticed it wasn't implemented and you asked for a repro at #163 (comment)

I had to create two files to test: I called them omit.go:

package codec

type TestSliceInt struct {
    I []int `json:",omitempty"`
}

type TestInnerSliceInt struct {
    T TestSliceInt `json:",omitempty"`
}

omit_test.go:

package codec

import (
    "testing"
)

func TestJsonEncodeOmitempty(t *testing.T) {
    var v TestInnerSliceInt
    jsonH := &JsonHandle{}
    jsonH.RecursiveEmptyCheck = true
    var bs []byte
    NewEncoderBytes(&bs, jsonH).MustEncode(&v)
    txt := string(bs)

    goldenResult := `{}`

    if txt != goldenResult {
	logT(t, "encoded != expected: \nexpected: %s\nencoded: %s", goldenResult, txt)
	failT(t)
    }
}

Run without code generation:

$ go test -v -tags netgo -run Omit
=== RUN   TestJsonEncodeOmitempty
--- PASS: TestJsonEncodeOmitempty (0.00s)
PASS
ok  	github.com/ugorji/go/codec	0.008s

Now with generation:

$ ./codecgen/codecgen -o omit_generated_test.go omit.go
$ go test -run Omit
--- FAIL: TestJsonEncodeOmitempty (0.00s)
	shared_test.go:277: encoded != expected: 
		expected: {}
		encoded: {"T":{}}
FAIL
exit status 1
FAIL	github.com/ugorji/go/codec	0.009s

(Note I probably wouldn't use this if it involved a lot of reflection; maybe if it worked via an interface like Selfer which allowed testing IsEmpty)

@ugorji
Copy link
Owner

ugorji commented Dec 12, 2017

Got it. Seen.

Yes, RecursiveEmptyCheck is not supported in codecgen. And it CANNOT be supported in codecgen.

Codecgen doesn't use reflection - as that gives up the performance it's after. RecursiveEmptyCheck uses reflection to introspect a struct and determine if empty.

For omitempty, we strive to statically determine whether a value is empty. Structs cannot be compared if non-pointer reference types exist (slices, maps, funcs, chans). We can't even compare them to the zero value (which should be ok, as it should be a comparison to nil values). So - the only recourse will be to enumerate the fields transitively, which is ... insane.

I remember trying it before, and punting as it was just ugly. E.g. for a struct like:

type T struct {
  I int 
  S []uint64
}

type T2 struct {
  F1 bool
  F2 map[int]float64
  F3 *T
  F4 T
}

var v T2 
v.F1 = &T{I: 10, S: []uint64{1,2,3}}

// the omitempty line for v would be:
var omit = v.F1 != false && v.F2 != nil && v.F3 != nil && v.F4.I != 0 && v.F4.S != nil

Let me think some more, and see if I can push this in. Writing it out, it doesn't seem as onerous anymore. But I reserve the right to still punt ;(

@bboreham
Copy link
Author

In my case I'm implementing Selfer for custom containers, so an extra interface to test emptiness will be easy.

I.e. you would generate

var omit = v.IsEmpty()

@ugorji
Copy link
Owner

ugorji commented Dec 12, 2017

I had gone back and forth regarding supporting an IsZero(), and decided not to do anything until the go community decides what they want to do about it.

See
golang/go#7501
https://go-review.googlesource.com/c/go/+/23064
golang/go#11939

So - we won't support IsEmpty or IsZero at this time. I don't want to do something unique for codec.

@ugorji ugorji closed this as completed in 8badb25 Dec 12, 2017
@ugorji
Copy link
Owner

ugorji commented Dec 12, 2017

I'm not too happy with this solution, as it ignores unexported fields, which many times are the real place where state is stored.

So - i am still considering using this as a forcing function to add "IsZero" support across the board for isEmpty.

ugorji added a commit that referenced this issue Dec 12, 2017
…rison of structs to zero value

With this, we now support using IsZero method on a type to determine if it's equal to the Zero value.
Also, if the type is comparable, we just compare it to the zero value directly.

Updates #224
@ugorji
Copy link
Owner

ugorji commented Dec 12, 2017

@bboreham

See updates via 8badb25 and then ae82d72 and then d36cb94 . These should solve your concerns completely i.e. the IsZero() support and comparable to efficiently compare to a zero value (if possible).

Let me know how it works.

balabit-sync pushed a commit to balabit-deps/balabit-os-9-etcd that referenced this issue Oct 3, 2023
etcd (3.3.25+dfsg-7ubuntu0.22.04.1) jammy-security; urgency=medium

  * No-change rebuild for golang-golang-x-text

etcd (3.3.25+dfsg-7) unstable; urgency=medium

  * Team upload.

  [ Shengjing Zhu ]
  * Switch autopkgtest to the new Architecture: field

  [ Reinhard Tartler ]
  * Avoid postinst crashes, Closes: #889714

etcd (3.3.25+dfsg-6) unstable; urgency=medium

  * Team upload.
  * Use packaged library
    + golang-github-grpc-ecosystem-go-grpc-middleware-dev
    + golang-github-soheilhy-cmux-dev
    + golang-github-tmc-grpc-websocket-proxy-dev

etcd (3.3.25+dfsg-5) unstable; urgency=medium

  * Team upload.
  * Upload to unstable
  * Rework goroutine leak patch
  * Disable parallel test to improve stability on slow arch
  * Adapt manpage and default config for 3.3 release

etcd (3.3.25+dfsg-4) experimental; urgency=medium

  * Team upload.
  * Not exit immediately on unsupported arch (Closes: #952536)
  * Add back some old patches to fix flaky tests

etcd (3.3.25+dfsg-3) experimental; urgency=medium

  * Team upload.
  * Backport patch to fix tls test failure

etcd (3.3.25+dfsg-2) experimental; urgency=medium

  * Team upload.
  * Fix tests failed with Ctty not valid in child.
    Address: #971158
  * Rewrite integration and functional tests in autopkgtest
  * Use execute_after_dh_auto_test
  * Update copyright
  * Fix file permission in golang-etcd-server-dev
  * Add Pre-Depends to etcd-server

etcd (3.3.25+dfsg-1) experimental; urgency=medium

  * Team upload.
  * New upstream release 3.3.25
    + CVE-2020-15136 (Closes: #968752)
      Gateway TLS authentication only applies to endpoints detected in DNS SRV
      records
      GHSA-wr2v-9rpq-c35q
    + CVE-2020-15115 (Closes: #968740)
      No minimum password length
      GHSA-4993-m7g5-r9hh
    + CVE-2020-15114
      Gateway can include itself as an endpoint resulting in resource
      exhaustion
      GHSA-2xhq-gv6c-p224
    + CVE-2020-15113
      Directories created via os.MkdirAll are not checked for permissions
      GHSA-chh6-ppwq-jh92
    + CVE-2020-15112
      An entry with large index causes panic in WAL ReadAll method
      GHSA-m332-53r6-2w93
    + CVE-2020-15106
      A large slice causes panic in decodeRecord method
      GHSA-p4g4-wgrh-qrg2
  * Disable some failed tests (Closes: #971158)
  * Bump debhelper compat to 13
  * Add Rules-Requires-Root
  * Bump Standards-Version to 4.5.0 (no changes)

etcd (3.2.26+dfsg-8) unstable; urgency=medium

  * Team upload.
  * debian/patches/embed_tests_fix.patch: New patch, skips
    TestStartEtcdWrongToken test which fails if a etcd server is already
    running independently of the testsuite.

etcd (3.2.26+dfsg-7) unstable; urgency=medium

  * Team upload.
  * debian/rules: Provide an explicit and PHONY build rule, as before
    `debian/rules build' was a no-op which broke autopkgtest-pkg-go
    integration.(Closes: #956424)
  * debian/patches/series: Reactivate and update skip-dev-ptmx-error.patch.
  * debian/patches/e2e_tests_fix.patch,
    debian/patches/functional_tests_fix.patch,
    debian/patches/integration_tests_fix.patch: New patches, fix issues in the
    integration tests.
  * debian/rules: Add new rule autopkgtest which runs the integration tests
    during autopkgtest, as defined by all tests which are not part of
    INTEGRATION_TEST_EXCLUDES.

etcd (3.2.26+dfsg-6) unstable; urgency=medium

  * Team upload.
  * Rebuild using newer golang-golang-x-net-dev

etcd (3.2.26+dfsg-5) unstable; urgency=medium

  * Team upload.
  * Add patch to fix FTBFS with newer prometheus version (Closes: #947939)
  * Build-Depends on golang-github-prometheus-client-golang-dev (>= 1.0.0~)

etcd (3.2.26+dfsg-4) unstable; urgency=medium

  * Team upload.

  [ Arnaud Rebillout ]
  * Add upstream patch to build against golang-google-grpc-dev in sid.
  * Add upstream patch to fix test with go 1.12.

  [ Dmitry Smirnov ]
  * (Build-)Depends:
    - golang-github-dgrijalva-jwt-go-v3-dev
    + golang-github-dgrijalva-jwt-go-dev (>= 3.2.0~)

etcd (3.2.26+dfsg-3) unstable; urgency=medium

  * Team upload.
  * Add patch to increase the latency in test to support mips
  * Fix override_dh_auto_test-does-not-check-DEB_BUILD_OPTIONS
  * Don't ship etcd2-backup-coreos, which is intended for coreos
  * Add spelling-error.patch
  * Remove obsoleted lintian-overrides
  * Update etcd and etcdctl manpage for 3.2.26 version

etcd (3.2.26+dfsg-2) unstable; urgency=medium

  * Team upload.
  * Add missing systemd service after compat bumped to 11
  * Only run unit tests when building, and don't ignore the results
  * Add patch to fix goroutine leak in TestDialNoTimeout

etcd (3.2.26+dfsg-1) unstable; urgency=medium

  * Team upload.

  [ Arnaud Rebillout ]
  * {Build-,}Depends on golang-github-xiang90-probing-dev (>= 0.0.1~)
  * Build-Depends on golang-any (>= 2:1.10~)

  [ Shengjing Zhu ]
  * New upstream release v3.2.26
    + Address CVE-2018-16886 (Closes: #923008)
      Disable CommonName authentication for gRPC-gateway
      gRPC-gateway proxy requests to etcd server use the etcd
      client server TLS certificate. If that certificate contains
      CommonName we do not want to use that for authentication as
      it could lead to permission escalation.
  * Remove pgpsigurlmangle in debian/watch.
    Upstream didn't sign the source tarball since v3.2.26
  * Update pkg-go team address to team+pkg-go@tracker.debian.org
  * Update debhelper and compat to 11
  * Update etcd server default env from upstream docs
  * Remove etcd-dump-db, etcd-dump-logs in etcd-client package
    upstream didn't provide these tools in v3.2.26 tarball
  * Add golang-go.uber-zap-dev to {Build-,}Depends
  * Remove socket files created during test phase

etcd (3.2.18+dfsg-1) unstable; urgency=medium

  [ Alexandre Viau ]
  * Point Vcs-* urls to salsa.debian.org.

  [ Anthony Fok ]
  * New upstream release.
  * Bump Standards-Version to 4.1.4 (no change)
  * Remove match-ugorji-go-codec-native-time.Time-support.patch
    because github.com/ugorji/go/codec@v1.1.1 includes changes to support
    both time.Time and *time.Time correctly for backward compatibility.
    See etcd-io/etcd#9447
  * {Build-,}Depends on golang-github-ugorji-go-codec-dev (>= 1.1.1~)

etcd (3.2.17+dfsg-1) unstable; urgency=medium

  * New upstream release.
  * Fix FTBFS:
     - New upstream release contains regenerated gRPC *.pb.go and *.pb.gw.po
       files (since etcd 3.2.10) which build correctly with the updated gPRC
       packages in Debian.
     - Add "export DH_GOLANG_GO_GENERATE := 1" to debian/rules
       to fix FTBFS by re-generating keys.generated.go at build time
       with the same version of codecgen as golang-github-ugorji-go-codec-dev.
       See also etcd-io/etcd#8715.
     - Add "Depends: golang-github-ugorji-go-codec" to have codecgen available
       at build time.
    (Closes: #890939)
  * Depend on golang-github-coreos-bbolt-dev, replacing
    golang-github-boltdb-bolt-dev, to "address backend database size
    issue" (since etcd 3.2.10)
  * Revert incoming-outgoing-context.patch (commit 5e059fd from upstream)
    which has been backported upstream in commit d62e39d from v3.3 branch
    to v3.2 branch since etcd 3.2.10
  * Add match-ugorji-go-codec-native-time.Time-support.patch, which updates
    etcd/client/keys{,_test}.go to match the latest
    golang-github-ugorji-go-codec-dev to prevent a new "cannot use
    x.Expiration (type *time.Time) as type time.Time in argument to
    r.encDriver.EncodeTime" error, see ugorji/go#224
    and ugorji/go@8badb25.
  * Apply "cme fix dpkg" to debian/control,
    bumping Standards-Version to 4.1.3, setting Priority to optional,
    and adding Testsuite: autopkgtest-pkg-go, etc.
  * Add myself to the list of Uploaders

etcd (3.2.9+dfsg-3) unstable; urgency=medium

  * Team upload.
  * Exclude the sockets from the MD5 sum generation. (Closes: #855876)
  * Use Priority optional
  * Update team name
  * Use wrap-and-sort on debian files
  * Remove dh_golang and golang-any from Depends of source package
  * Use HTTPS URL for d/copyright
  * Switch to XS-Go-Import-Path in d/control

etcd (3.2.9+dfsg-2) unstable; urgency=medium

  * Team upload.
  * Fix package dependency typo. (Closes: #879791)

etcd (3.2.9+dfsg-1) unstable; urgency=medium

  * Team upload.

  [ Tim Potter ]
  * New upstream release.

  [ Paul Tagliamonte ]
  * Remove Built-Using from arch:all transitional package

  [ Tim Potter ]
  * Apply commit 5e059fd from upstream
  * Update test fixture modification patch
  * Update /dev/ptmx input/output error patch
  * Another update to test fixture path patch

  [ Andrew Shadura ]
  * Ignore test failures.

etcd (3.1.8+dfsg-2) unstable; urgency=medium

  * Fix upgrade problem caused by client/server package split. Thanks
    to Olafur St. Arnarsson for the patch. (Closes #863976)

etcd (3.1.8+dfsg-1) unstable; urgency=medium

  * New upstream release.
  * Tighten B-D on golang-github-coreos-pkg-dev to build in stretch.
    (Closes: #858241)
  * Change section from "devel" to "net. (Closes: #840681)
  * Separate into client and server packages. (Closes: #815453)

etcd (3.1.4+dfsg-1) unstable; urgency=medium

  * New upstream release.

etcd (3.1.3+dfsg-1) unstable; urgency=medium

  * New upstream release.
  * Suppress binary-without-manpage Lintian warnings.

etcd (3.1.2+dfsg-1) unstable; urgency=medium

  * New upstream release.

etcd (3.1.1+dfsg-2) unstable; urgency=medium

  * Upload to unstable. (Closes: #846542)
  * Fix test suite failures in Debian build environment and
    ensure no test junk is accidentally mispackaged. (Closes: #855876)
  * Fix various Lintian problems.

etcd (3.1.1+dfsg-1) experimental; urgency=medium

  * New upstream release.

etcd (3.1.0-1) experimental; urgency=medium

  * New upstream release.
  * Change to use upstream source generated by dh-make-golang, and
    remove antiquated package build system.
  * Update debian/copyright file for new major release.
  * Build-Depends:
      + golang-github-karlseguin-ccache-dev
      + golang-github-cockroachdb-cmux-dev
      + golang-github-urfave-cli-dev
      + golang-github-grpc-ecosystem-grpc-gateway-dev
      + golang-github-grpc-ecosystem-go-grpc-prometheus-dev
  * Add lsb-base as install dependency for binary package.
  * Remove bogus Build-Depends from -dev binary package.

etcd (2.3.7+dfsg-5) unstable; urgency=medium

  [ Team upload ]
  * Regenerate pb.go files with gogo-protobuf v0.3 (Closes: #835750)
  * Patch to fix SdNotify() API change in coreos-go-systemd package.

etcd (2.3.7+dfsg-4) unstable; urgency=medium

  * New patch to disable TestTransportErrorc (Closes: #831789).

etcd (2.3.7+dfsg-3) unstable; urgency=medium

  * New patch to disable "TestLessorRevoke" test.

etcd (2.3.7+dfsg-2) unstable; urgency=medium

  * Tests: disabled "TestWaitTime" test due to failure on [ppc64el].
  * Renamed "cheggaaa-pb-dev" to "cheggaaa-pb.v1-dev" (Closes: #829048).
    Also commented currently unused package. Thanks, Peter Colberg.
  * (Build-)Depends: removed obsolete -clockwork-dev alternative
    (Closes: #830388).

etcd (2.3.7+dfsg-1) unstable; urgency=medium

  * New upstream release [June 2016].
  * Build-Depends += "curl".
  * Disabled failing tests; don't ignore test failures any more.

etcd (2.3.6+dfsg-1) unstable; urgency=medium

  * New upstream release [May 2016].

etcd (2.3.5+dfsg-1) unstable; urgency=medium

  * New upstream release [May 2016].
  * Fixed Vcs-Git URL.

etcd (2.3.3+dfsg-1) unstable; urgency=medium

  * New upstream release [April 2016].
  * Standards-Version: 3.9.8.

etcd (2.3.2+dfsg-1) unstable; urgency=medium

  * New upstream release [April 2016].
  * control: drop Built-Using from -dev package.
  * Build-Depends:
    = golang-github-boltdb-bolt-dev (>= 1.2.0~)

etcd (2.3.1+dfsg-1) unstable; urgency=medium

  * New upstream release [April 2016].
  * Removed obsolete "rakyll2cheggaaa.patch".
  * Standards-Version: 3.9.7.
  * Fix "readlink" invocation in the init.d script.
  * rules: correction to build with gogoprotobuf 0.2.
  * Build-Depends:
    - golang-etcd-dev | golang-github-coreos-go-etcd-dev
    + golang-github-bgentry-speakeasy-dev
    + golang-github-codegangsta-cli-dev (>= 0.0~git20151221~)
    + golang-github-coreos-gexpect-dev
    + golang-github-gogo-protobuf-dev (>= 0.2~)
    + gogoprotobuf
    + golang-github-mattn-go-runewidth-dev
    + golang-github-olekukonko-tablewriter-dev
    + golang-github-xiang90-probing-dev

etcd (2.2.5+dfsg-1) unstable; urgency=medium

  * New upstream release [February 2016] (Closes: #814404).
  * Build-Depends:
    - golang-github-bradfitz-http2-dev
    - golang-golang-x-oauth2-dev
    - golang-google-cloud-compute-metadata-dev
    - golang-google-grpc-dev
    + golang-google-grpc-dev (>= 0.0~git20151002~)
    + golang-github-akrennmair-gopcap-dev
    + golang-pb-dev | golang-github-cheggaaa-pb-dev
    + golang-github-coreos-pkg-dev
    + golang-github-cpuguy83-go-md2man-dev
    + golang-github-kballard-go-shellquote-dev
    + golang-pty-dev
    + golang-github-russross-blackfriday-dev
    + golang-github-shurcool-sanitized-anchor-name-dev
    + golang-github-spacejam-loghisto-dev
    + golang-github-spf13-cobra-dev
    + golang-github-spf13-pflag-dev
  * Updated Vcs URLs (vcs-field-uses-insecure-uri).
  * Switch to bundled "github.com/gogo/protobuf".

etcd (2.2.3+dfsg-1) unstable; urgency=medium

  * New upstream release [December 2015].
  * etcd.service: add "Alias=etcd2.service".

etcd (2.2.2+dfsg-2) unstable; urgency=medium

  * Added goland dependencies to -dev package Depends.

etcd (2.2.2+dfsg-1) unstable; urgency=medium

  * New upstream release [November 2015].
  * Build-Depends:
    + golang-github-ugorji-go-codec-dev (>= 0.0~git20151112~).
    + golang-github-beorn7-perks-dev
  * init.d: log to syslog.
    + Depends += "pipexec".

etcd (2.2.1+dfsg-1) unstable; urgency=medium

  * New upstream release [October 2015].
    - switch to bundled "github.com/ugorji/go" due to FTBFS.
  * Un-bundled "golang-google-grpc-dev".
  * Allow Etcd to notify systemd for readiness through service "Type=notify"
    (Closes: #800646). Thanks, Matthias Urlichs.
  * Dropped obsolete lintian-overrides.
  * Corrected "duplicate-short-description".

etcd (2.2.0+dfsg-2) unstable; urgency=medium

  * Build-Depends: swap alternatives to put non-existent packages last.
  * .service: less aggressive restart (on-failure --> on-abnormal).

etcd (2.2.0+dfsg-1) unstable; urgency=medium

  [ Tianon Gravi ]
  * Update a few old-style "golang-" Build-Depends values to use "|" with
    their new proper names to help with transitioning the dependencies.
    See https://bugs.debian.org/797903#10, for example.

  [ Dmitry Smirnov ]
  * New upstream release [September 2015].
  * Added etcd(1) man page.
  * Sorted list of packages in Build-Depends.
  * Build-Depends:
    + golang-github-coreos-go-systemd-dev
    + golang-golang-x-sys-dev
    - golang-mreiferson-httpclient-dev (unused).
    + golang-google-cloud-compute-metadata-dev
  * Provides:
    - golang-github-coreos-go-etcd-dev
    + golang-github-coreos-etcd-dev
  * rules: --parallel.
  * New annotated .default file "/etc/default/etcd".
  * Re-written init scripts.

etcd (2.1.3+dfsg2-1) unstable; urgency=medium

  * New upstream release [September 2015].
  * Build-Depends:
    + golang-golang-x-net-dev
    + golang-protobuf-extensions-dev
  * Provides: "golang-github-coreos-go-etcd-dev" (policy-compliant package).
  * Copyright: more Files-Excluded.
  * Added "etcdctl.1" man page.

etcd (2.1.2+dfsg1-1) unstable; urgency=medium

  * Upload to unstable (Closes: #788762).

  [ Jelmer Vernooij ]
  * Drop dependency on golang-raft, which is no longer used.

  [ Dmitry Smirnov ]
  * New upstream Release [August 2015].
  * Re-build .pb.go files.
  * control: updated Vcs-Browser URL.
  * postinst: check if user exist.
  * systemd/init.d/default:
    + use hostname as default instance name.
    + consistently load ETCD_NAME and DATA_DIR.
  * init.d:
    + added LSB descriptions.
    + added "status" support.
    + replaced "echo" with LSB functions.
    + stop daemon with "--retry=TERM/30/KILL/5" to fix restart.
  * rules:
    + always run tests but ignore failures.
    + invoke DH --with systemd.
  * rules/override_dh_clean: remove Files-Excluded.
  * cleanup_third_party: limit find scope for a little speed-up.
  * Build-Depends:
    - golang-gogoprotobuf-dev
    + golang-gogoprotobuf-dev (>= 0.0~git20150828~)
    + libprotobuf-dev
    + protobuf-compiler
    + golang-clockwork-dev
    + golang-procfs-dev
    + golang-github-bradfitz-http2-dev
    + golang-github-boltdb-bolt-dev
    + golang-github-google-btree-dev
    + golang-github-ugorji-go-codec-dev
    + golang-glog-dev
    + golang-go-semver-dev
    + golang-prometheus-client-dev
    + golang-go.crypto-dev
    + golang-golang-x-oauth2-dev
  * Added myself to Uploaders.

etcd (2.0.8-2) experimental; urgency=medium

  * Add support for setting up SSL in default config.
  * Fix service stop.
  * debian/rules: run tests on amd64.

etcd (2.0.8-1) experimental; urgency=medium

  * New upstream release.
  * Bump standards version to 3.9.6 (no changes).
  * Add patch 01_race_amd64: only specify --race to test on amd64.

etcd (2.0.0-1) experimental; urgency=medium

  * Initial release. (Closes: #741065)
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

2 participants