Hi Caleb, On 08/19/2010 10:40 AM, Caleb James DeLisle wrote: > > > Sergiu Dumitriu wrote: >> On 08/18/2010 11:49 PM, Caleb James DeLisle wrote: >>> >>> Sergiu Dumitriu wrote: >>>> On 08/18/2010 04:43 PM, Anca Luca wrote: >>>>> Hi Caleb, >>>>> >>>>> On 08/17/2010 09:15 PM, Caleb James DeLisle wrote: >>>>>> I am going to commit this change and want to continue to discuss >>>>>> possible side effects. >>>>>> >>>>>> I think this change will solve the bug causing database corruption when >>>>>> list property is switched to relational storage >>>>>> (I can't find this in jira anyone know the number? Sergiu?) >>>>>> >>>>>> I will be adding more tests and am ready to revert it at the first sign >>>>>> of trouble. >>>>> I haven't had time to look at the issue to be able to cast a vote >>>>> knowing what I am saying but >>>>> >>>>> i don't agree with this approach. please don't. >>>> One thing that will break is HQL queries hardcoded on StringProperty. >>> I thought about this but it seemed to be acceptable since the only time >>> anything would break is when the input >>> exceeded 255 chars and in such cases the old behavior was to blow up with a >>> database error. >> >> Well, not quite. >> >> Before: >> >> X edits a document, inputs a long value, tries to save: error (not the >> ugly exception that gets thrown now, but a nice error message which >> mentions the problem nicely). User inputs a shorter text, saves, >> everyone is happy. >> >> After: >> >> X edits a document, inputs a long value, saves, but now the livetable >> which lists that kind of documents doesn't filter or order correctly on >> that column anymore. > > I'm not totally convinced that this change is a good one and since you and > Anca disagree with it, I'll > pursue other avenues for long password hash handling.
My disagreement was for the "i'm gonna commit now and rollback after if anything" approach. In the case of this change, I don't like it since it can be a quite hidden side effect which we discover too much later. I am still to investigate whether the loads & saves handle well this situation, since I've seen bad things happening because of setting properties on objects with setLargeString() when they were anything but. However Sergiu's point is a good one, the question is if we're willing to pay this price (and break bw compat). Thanks, Anca > > > Caleb > >> >>>>> Thanks, >>>>> Anca >>>>> >>>>>> Caleb >>>>>> >>>>>> >>>>>> Caleb James DeLisle wrote: >>>>>>> I have what I think is a better solution. >>>>>>> I have found that I can replace the StringProperty objects with >>>>>>> LargeStringProperty in >>>>>>> XWikiHibernateStore and they will save and load ok. >>>>>>> I have a patch http://jira.xwiki.org/jira/browse/XWIKI-5415 and would >>>>>>> be interested to >>>>>>> hear what others have to say. In the mean time I will work on adding >>>>>>> automated tests >>>>>>> to prove that load save and search continue to work. >>>>>>> >>>>>>> Caleb >>>>>>> >>>>>>> Thomas Mortagne wrote: >>>>>>>> +0 >>>>>>>> >>>>>>>> Le 2010 8 10 19:34, "Caleb James DeLisle"<[email protected]> >>>>>>>> a >>>>>>>> écrit : >>>>>>>>> Because protectPassword generates a base-64 encoded java serialized >>>>>>>>> form, >>>>>>>> the size is quite a bit larger than >>>>>>>>> the 255 character limit of StringProperty and thus PasswordProperty. >>>>>>>>> >>>>>>>>> The use of java serialization is central to the upgradability of the >>>>>>>> password verification function because >>>>>>>>> any new class which implements PasswordVerificationFunction >>>>>>>>> automatically >>>>>>>> works. >>>>>>>>> Given this, I want to migrate the database to move password hashes >>>>>>>>> into >>>>>>>> the xwikilargestrings table and change >>>>>>>>> PasswordProperty to extend LargeStringProperty. During this >>>>>>>>> migration, any >>>>>>>> passwords still stored in plaintext >>>>>>>>> will be ported to the scrypt function, passwords stored as a hash will >>>>>>>> have an exclamation mark pretended to the >>>>>>>>> text (this is invalid base64) and be inserted into the table as is. >>>>>>>>> >>>>>>>>> PasswordClass will keep the sha-512 hash function for legacy >>>>>>>>> passwords but >>>>>>>> will port passwords to the new format >>>>>>>>> as users log in. >>>>>>>>> >>>>>>>>> These changes will allow us to close >>>>>>>>> http://jira.xwiki.org/jira/browse/XWIKI-70 >>>>>>>>> and >>>>>>>>> http://jira.xwiki.org/jira/browse/XWIKI-582 >>>>>>>>> >>>>>>>>> >>>>>>>>> WDYT? >>>>>>>>> >>>>>>>>> >>>>>>>>> Caleb >> >> > > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
