[Blama]

Hi all,

I added a possible related testcase here:
http://forums.smartclient.com/showthread.php?p=84986

Best regards,
Blama

[davidj6]

A fix for the extra validation request has been checked in and should be in the next nightly build.

[nikolayo]

Sorry if I got carried away.
Thank you for your patience and advice.

Regards
Nikolay

[Isomorphic]

Your confused about what we're saying - please re-read post 14 and keep in mind two separate things are being discussed.

It makes sense to use oldValues for optimistic concurrency checking. This is what we've been saying all along.

It does not make sense to use oldValues for a data validity check, such as making sure two fields have values that make sense together. This could allow a user to corrupt data or bypass security restrictions.

For example, if the business rule being enforced is that you shouldn't be allowed to modify shipDate when order.status is "shipped", if you rely on the oldValues submitted by the user to check order.status, then the user can still modify the shipDate by sending oldValues which falsely show order.status as "submitted" or whatever other value.

Most implementations of optimistic concurrency, including the one you outlined, would not prevent this. The user can easily submit oldValues which include a correct version value but incorrect values for other fields. The only optimistic concurrency implementations that would happen to catch this would be ones where the version value is a hash of the values of the other fields, or where all fields are always checked.

This is why we don't pass oldValues to server-side validation. If we were to make such data available, we'd provide data fetched dynamically from the database instead. But as the above samples show, we already make that very very easy to do.

[nikolayo]

I do not quite understand why you single out old values. User can in theory control any/all parts of the request but so what? Unless of course we assume a malicious user. But if a malicious user manages to obtain data modification rights then we are screwed anyway regardless of validation strategy.

Optimistic locking is a fairly standard technique, I think. Every record has a version which changes in an atomic way with every successful update. Version value as retrieved from the DB is included in the where clause of update request together with the primary key, so update will not happen if version in the DB is changed by concurrent modification. Reaction to this may vary somewhat by implementation. In my specific implementation user is notified that operation failed because another user modified the data and is advised to reload it (I prefer to not do that automatically, so user feels she is in control). Similar policy applies to delete operations too. With such arrangement I am not too much concerned about validation against old values which may be stale because update is not going to succeed with stale data anyway.

So all in all I would prefer the API to let me decide whether when and how to use old data but of course at the end of the day it is your decision.

Regards
Nikolay


Regards
Nikola

[Isomorphic]

Not sure what you mean by that term. Normally, it means that no DB locking is performed and updates are checked for conflicts when they are submitted. If you are using the term in this way, it has nothing to do with what we have said.

To recap:
1. oldValues allow you to detect conflicts, but the user can spoof them.
2. if, when a conflict is detected, the user is given a choice to proceed with their changes despite the conflict (this is common), then it is not a security concern that the user can spoof oldValues. The user is allowed to make the same update anyway.

[nikolayo]

Hmm, how would the user do that with optimistic locking in place?

Regards
Nikolay

[Isomorphic]

? You may have misunderstood, what we stated was not really a "position" or opinion. If the user is free to update the data even after concurrent modification has been detected, then it's not important if the user is spoofing oldValues. He can already do what he wants to do without spoofing.

Thanks for the test case, we'll take a look.

[nikolayo]

Thank you, Isomorphic. I recognize the validity of your position even though I do not share it. I deal with concurrency by automated optimistic locking implemented in a custom data source and if I can not trust the values from the client for some other reason, I will rather not allow updates all.

There are of course workarounds which bypass this design and so far I think I will be using some but I will also give that a second thought based on your concerns.

On the issue of redundant validate requests, here is a modified worldDS,ds.xml from the EE showcase :

<DataSource
    ID="worldDS"
    serverType="sql"
    tableName="worldDS"
    testFileName="/ds/test_data/world.data.xml"
>
    <fields>
        <field name="pk"            type="sequence"   hidden="true"            primaryKey="true" />
        <field name="countryCode"   type="text"       title="Code"             required="true"   />
        <field name="countryName"   type="text"       title="Country"          required="true"   />
        <field name="capital"       type="text"       title="Capital"          >
		<validators>
			<validator type="isUnique"/>
		</validators>
        </field>
        <field name="government"    type="text"       title="Government"       length="500"      />
        <field name="continent"     type="text"       title="Continent"        >
            <valueMap>
                <value>Europe</value>
                <value>Asia</value>
                <value>North America</value>
                <value>Australia/Oceania</value>
                <value>South America</value>
                <value>Africa</value>
            </valueMap>
        </field>
        <field name="independence"  type="date"       title="Nationhood"          />
        <field name="area"          type="float"      title="Area (km&amp;sup2;)" />
        <field name="population"    type="integer"    title="Population"          />
        <field name="gdp"           type="float"      title="GDP ($M)"            />
    </fields>
</DataSource>

With this data source and no modifications to code I changed the capital of Bermuda through the "Basic Connector" example. Here are the payloads of the requests that I observed in a Tomcat log (it seems the forum does not like catalina.out format, so I can not provide the complete log) :

{
    values:{
        countryName:"Bermuda",
        area:50,
        gdp:1700,
        continent:"North America",
        countryCode:"BD",
        independence:new Date(1225670400000),
        government:"dependent territory of the UK",
        capital:"Whatever",
        population:62099,
        pk:0,
        _selection_5:true
    },
    operationConfig:{
        dataSource:"worldDS",
        operationType:"validate"
    },
    validationMode:"partial",
    appID:"builtinApplication",
    operation:"worldDS_validate",
    oldValues:{
        countryName:"Bermuda",
        area:50,
        gdp:1700,
        continent:"North America",
        countryCode:"BD",
        independence:new Date(1225670400000),
        government:"dependent territory of the UK",
        capital:"Whatever",
        population:62099,
        pk:0,
        _selection_5:true
    },
    criteria:{
    }
}

{
    criteria:{
        pk:0
    },
    values:{
        pk:0,
        capital:"Whatever"
    },
    operationConfig:{
        dataSource:"worldDS",
        operationType:"update"
    },
    componentId:"isc_ListGrid_0",
    appID:"builtinApplication",
    operation:"worldDS_update",
    oldValues:{
        countryName:"Bermuda",
        area:50,
        gdp:1700,
        continent:"North America",
        countryCode:"BD",
        independence:new Date(1225670400000),
        government:"dependent territory of the UK",
        capital:"Hamilton",
        population:62099,
        pk:0
    }
}

Regards
Nikolay

[Isomorphic]

dsRequest.oldValues is sent from the client, hence it is spoofable - it is intended for the scenario of checking whether there have been concurrent edits when the user has the ability to proceed with the save anyway.

For a data validity or security check, you want to get the latest values from the database, which is secure. These two samples show doing a related DSRequest in the context of server-side validation.

On your issue with extra requests - can you actually take that isUnique validator and add it to a sample that comes with the SDK and reproduce the issue you're talking about? Because a lot of other settings could be involved here - the clearest thing would be if this can be reproduced in a sample.

[nikolayo]

Any advice on this?

Regards
Nikolay

[nikolayo]

Sorry about discussing 2 distinct issues at the same time but let me just in case and in addition to the post above summarize also the second issue under discussion so it does not get lost:

Server side custom vaidator method has generally the following signature:

    public boolean condition(Object value, Validator validator,
                             String fieldName, Map record)
    // Optional arguments:
    //
    // DataSource          dataSource,
    // RequestContext      requestContext,
    // HttpServletRequest  httpServletRequest,
    // HttpServletResponse servletResponse,
    // ServletContext      servletContext,
    // HttpSession         httpSession
    // RPCManager          rpcManager

The value to be validated is in the "value" argument. In order to do the validation I may need to compare it to the value of another field. The natural place to look for that second field is the "record" argument. Upon an update operation though "record" contains only fields whose value has changed. So how do I get access to the field I need if it's value has not changed?

This problem applies also to validation by velocity expressions because I observe exceptions in such validators under similar conditions. For example the following validator:

<field 
	name="END_DATE" 
	type="date">
	<validators>
		<validator
		type="serverCustom"
		errorMessage="End date must be after start date!"
		serverCondition="$value.after($record.START_DATE)"/>
	</validators>
</field>

throws an exception upon update operation if the value of the value of START_DATE was not changed.

Regards
Nikolay

[nikolayo]

I found out that apparently redundant validation requests to the server are issued by the client whenever there is a server-side validator (be it built-in or serverCustom) defined for a field and the value of that field has been changed during the editing of a grid record. This also applies to creation of new records. My previous observation re extension objects turned out to be irrelevant.

To reproduce this it is sufficient to put on an arbitrary field e.g. the following validator (other validator types work as well) :

            <validators>
                <validator type="isUnique"/>
            </validators>

Then change the value of the field in a grid record and finish editing. Or/and create a new record.

[nikolayo]

Sure. Upon update operation server side validator gets a "Map record" argument which contains the primary key and the changed values but does not contain the values which were not changed and need not be updated. The problem with this is that I may need to compare a changed value with a value which was not change in order to validate the changed value.

[Isomorphic]

Having trouble following what you mean about unchanged values and required validators - can you rephrase?