From dev-return-30500-archive-asf-public=cust-asf.ponee.io@geode.apache.org Fri Dec 21 23:54:57 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 754D3180627 for ; Fri, 21 Dec 2018 23:54:56 +0100 (CET) Received: (qmail 47446 invoked by uid 500); 21 Dec 2018 22:54:55 -0000 Mailing-List: contact dev-help@geode.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@geode.apache.org Delivered-To: mailing list dev@geode.apache.org Received: (qmail 47435 invoked by uid 99); 21 Dec 2018 22:54:54 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 21 Dec 2018 22:54:54 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id 82CD7C068B for ; Fri, 21 Dec 2018 22:54:54 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -0.001 X-Spam-Level: X-Spam-Status: No, score=-0.001 tagged_above=-999 required=6.31 tests=[RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001] autolearn=disabled Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id KwPDmy6R-Tvh for ; Fri, 21 Dec 2018 22:54:52 +0000 (UTC) Received: from mx0a-00296801.pphosted.com (mx0a-00296801.pphosted.com [148.163.150.38]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTPS id D90C35F583 for ; Fri, 21 Dec 2018 22:54:51 +0000 (UTC) Received: from pps.filterd (m0114582.ppops.net [127.0.0.1]) by mx0a-00296801.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id wBLMpGMV028705 for ; Fri, 21 Dec 2018 22:54:50 GMT Received: from mail-oi1-f198.google.com (mail-oi1-f198.google.com [209.85.167.198]) by mx0a-00296801.pphosted.com with ESMTP id 2pcrc5dxt3-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Fri, 21 Dec 2018 22:54:50 +0000 Received: by mail-oi1-f198.google.com with SMTP id w128so4333145oie.20 for ; Fri, 21 Dec 2018 14:54:49 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:references:subject:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=Tc/ey0tlhg82q2h9mqucULWEK6SnW+gOKUcPA3IHRuA=; b=dtabXKXU3TI9PqW99A/9AhGFzae7fOyXLypFZzsLXd/ag2dTQIQpRogJLhGWCDuoPz 8q7bNUj9LucQdzediaibE1ZehnJkMvkf1ZlgiJF59H4TFzVTWHaQBRYJdnlPGvBazhEY o6a1gEGkjfOWSRX7t7RUN8F2SY79UNC09tsR+SWD6YGEADzv6Z6qQvg35XvhWMIMrKfR uIxFp2/5+KZCjMcO2P+N0e6fTvuEnvxBcvIz09dH74WcwptL2jnKjfz6/635AQszQ/7u 0/yHuLkyFZqcPwYos/9b8vNkkqwRkCtPXBVIEh8rICgWd7xW84VxdDal1Nzf3tLxn0za A2xg== X-Gm-Message-State: AJcUukcZorf78v6vMWsytFunp9g2mXUotJri01w2kycgWlTxN2gnu5Fm qjgC4YcGvIQ4qWvT+mCZ409B3Ti6PN8H3XYlMyk74RwqnQGsWhr58FGIYAFattLTyUQEBFM7Qy+ IDTYKwUTik5ZAjFP0Y6W/XHyRByGHK1+xJ5O48A== X-Received: by 2002:a05:6808:97:: with SMTP id s23mr2480506oic.251.1545432888982; Fri, 21 Dec 2018 14:54:48 -0800 (PST) X-Google-Smtp-Source: AFSGD/UchfOuEhaj6Wf67Ft5j/9z+36WFSNlEHacbT5w0WlJ8ftjIdvQovcLkKfhi8mpXmE0cXY0Lw== X-Received: by 2002:a05:6808:97:: with SMTP id s23mr2480496oic.251.1545432888553; Fri, 21 Dec 2018 14:54:48 -0800 (PST) Received: from [192.168.1.218] ([173.84.51.84]) by smtp.gmail.com with ESMTPSA id o16sm12064205otl.22.2018.12.21.14.54.47 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 21 Dec 2018 14:54:47 -0800 (PST) To: dev@geode.apache.org References: Subject: Re: [PROPOSAL] use default value for validate-serializable-objects in dunit From: Bruce Schuchardt Message-ID: Date: Fri, 21 Dec 2018 14:54:46 -0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0) Gecko/20100101 Thunderbird/60.3.3 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-12-21_10:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=696 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1810050000 definitions=main-1812210170 -1 I'm fearful that removing full testing of serialization validation leaves the project exposed to java-serialization bombs.  We need to ensure that our servers and clients are protected from malicious deserialization attacks. On 2018/12/21 22:42:23, Kirk Lund wrote: > > > > I filed GEODE-6202: DUnit should not enable VALIDATE_SERIALIZABLE_OBJECTS> > by default> > https://issues.apache.org/jira/browse/GEODE-6202> > > And submitted PR #3023> > https://github.com/apache/geode/pull/3023> > > Please review and/or discuss further if needed.> > > Thanks,> > Kirk> > > On Thu, Mar 15, 2018 at 12:00 PM Bruce Schuchardt > > wrote:> > > > +0.5 I think we can turn this off (back to the default) now since the> > > AnalyzeSerializables tests for other modules have been created. These> > > tests ensure that serializable objects are properly white-listed or> > > excluded and are able to be serialized/deserialized.> > >> > > Excluded classes are not tested, though, so if you add an excluded class> > > and are wrong about whether it is sent over the wire you will break> > > Geode. Having serialization validation enabled in dunit tests protects> > > against this if you write tests that use the excluded class.> > >> > > We also have non-default values for cluster configuration in dunit runs.> > >> > >> > > On 3/13/18 10:03 AM, Kirk Lund wrote:> > > > I want to propose using the default value for> > > validate-serializable-object> > > > in dunit tests instead of forcing it on for all dunit tests. I'm> > > > sympathetic to the reason why this was done: ensure that all existing> > > code> > > > and future code will function properly with this feature enabled.> > > > Unfortunately running all dunit tests with it turned on is not a good way> > > > to achieve this.> > > >> > > > Here are my reasons:> > > >> > > > 1) All tests should start out with the same defaults that Users have. If> > > we> > > > don't do this, we are going to goof up sometime and release something> > > that> > > > only works with this feature turned on or worsen the User experience of> > > > Geode in some way.> > > >> > > > 2) All tests should have sovereign control over their own configuration.> > > We> > > > should strive towards being able to look at a test and determine its> > > config> > > > at a glance without having to dig through the framework or other classes> > > > for hidden configuration. We continue to improve dunit in this area but I> > > > think adding to the problem is going in the wrong direction.> > > >> > > > 3) It sets a bad precedent. Do we follow this approach once or keep> > > adding> > > > additional non-default features when we need to test them too? Next one> > > is> > > > GEODE-4769 "Serialize region entry before putting in local cache" which> > > > will be disabled by default in the next Geode release and yet by turning> > > it> > > > on by default for all of precheckin we were able to find lots of problems> > > > in both the product code and test code.> > > >> > > > 4) This is already starting to cause confusion for developers thinking> > > its> > > > actually a product default or expecting it to be enabled in other> > > > (non-dunit) tests.> > > >> > > > Alternatives for test coverage:> > > >> > > > There really are no reasonable shortcuts for end-to-end test coverage of> > > > any feature. We need to write new tests or identify existing tests to> > > > subclass with the feature enabled.> > > >> > > > 1) Subclass specific tests to turn validate-serializable-object on for> > > that> > > > test case. Examples of this include a) dunit tests that execute Region> > > > tests with OffHeap enabled, b) dunit tests that flip on HTTP over GFSH,> > > c)> > > > dunit tests that run with SSL or additional security enabled.> > > >> > > > 2) Write new tests that cover all features with> > > validate-serializable-object> > > > enabled.> > > >> > > > 3) Rerun all of dunit with and without the option. This doesn't sound> > > very> > > > reasonable to me, but it's the closest to what we really want or need.> > > >> > > > Any other ideas or suggestions other than forcing all dunit tests to run> > > > with a non-default value?> > > >> > > > Thanks,> > > > Kirk> > > >> > >> > >> >