Anca Luca wrote: > 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.
I had mentioned it in an earlier email, with no response I replied and changed the subject line which then got more attention. I have done quite a bit of testing already but I think (as with any change) eventually one must take this 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. As a side note, XWikiHibernateStore seems to suffer from a lack of maintenance, it seems that everyone is afraid of breaking something. I think letting it deteriorate is a worse option than writing new code even if it will inevitably contain bugs. > 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. Sounds like the queries I was seeing where each property table is joined. > However Sergiu's point is a good one, the question is if we're willing > to pay this price (and break bw compat). As I said, I'm not attached to the idea, it has it's costs and benefits. I'm just happy that there is some dialogue going on around this so important part of the code. Caleb > > 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 _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
