You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flex.apache.org by Alex Harui <ah...@adobe.com> on 2017/04/09 04:48:20 UTC
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
IMO, with PAYG, this would go in an extension of HTTPService. Not all
apps will need CORS.
Thanks,
-Alex
On 4/8/17, 8:26 PM, "jmclean@apache.org" <jm...@apache.org> wrote:
>Repository: flex-asjs
>Updated Branches:
> refs/heads/develop 11ef21aae -> 326d69791
>
>
>CORS security. Allow auth credentials to be passed when using cross site
>calls. This is required as well as setting the
>Access-Control-Allow-Origin header on the server.
>
>
>Project:
>https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit-wip-us
>.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Frepo&data=02%7C01%7C%7C58994717190
>044f003a908d47ef83ba7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C6362730
>52005920291&sdata=eQwxowk79ikXeDxbqXV3OeVrXUzTXNfFR0eKzhU8wiw%3D&reserved=
>0
>Commit:
>https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit-wip-us
>.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Fcommit%2F326d6979&data=02%7C01%7C%
>7C58994717190044f003a908d47ef83ba7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%
>7C0%7C636273052005930299&sdata=f8udpkPpLcL1ivRV3LDW0kJARc8QnLhBDHVFGgCko7M
>%3D&reserved=0
>Tree:
>https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit-wip-us
>.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Ftree%2F326d6979&data=02%7C01%7C%7C
>58994717190044f003a908d47ef83ba7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C
>0%7C636273052005930299&sdata=t6MMifwasbm2bgCuzsVN2q4%2BCYcB2uB8o7O%2B%2BJu
>yZ5w%3D&reserved=0
>Diff:
>https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit-wip-us
>.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Fdiff%2F326d6979&data=02%7C01%7C%7C
>58994717190044f003a908d47ef83ba7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C
>0%7C636273052005930299&sdata=gJDrCW4YbwaorNFSXVCVjLyq3lwTC67VhCRLEtYlcD0%3
>D&reserved=0
>
>Branch: refs/heads/develop
>Commit: 326d69791b37cc2aaac546bcfcd3a51e88716f2f
>Parents: 11ef21a
>Author: Justin Mclean <jm...@apache.org>
>Authored: Sun Apr 9 13:26:30 2017 +1000
>Committer: Justin Mclean <jm...@apache.org>
>Committed: Sun Apr 9 13:26:30 2017 +1000
>
>----------------------------------------------------------------------
> .../src/main/flex/org/apache/flex/net/HTTPService.as | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>----------------------------------------------------------------------
>
>
>https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit-wip-us
>.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Fblob%2F326d6979%2Fframeworks%2Fpro
>jects%2FNetwork%2Fsrc%2Fmain%2Fflex%2Forg%2Fapache%2Fflex%2Fnet%2FHTTPServ
>ice.as&data=02%7C01%7C%7C58994717190044f003a908d47ef83ba7%7Cfa7b1b5a7b3443
>8794aed2c178decee1%7C0%7C0%7C636273052005930299&sdata=TcDMCOEVbLKxedpsnACV
>OmZon89YgUkvGxOtd%2F3Qky8%3D&reserved=0
>----------------------------------------------------------------------
>diff --git
>a/frameworks/projects/Network/src/main/flex/org/apache/flex/net/HTTPServic
>e.as
>b/frameworks/projects/Network/src/main/flex/org/apache/flex/net/HTTPServic
>e.as
>index b939751..3a9968c 100644
>---
>a/frameworks/projects/Network/src/main/flex/org/apache/flex/net/HTTPServic
>e.as
>+++
>b/frameworks/projects/Network/src/main/flex/org/apache/flex/net/HTTPServic
>e.as
>@@ -505,6 +505,18 @@ package org.apache.flex.net
> return null;
> }
>
>+ /**
>+ * Allows Javascript cross-site Access-Control requests to be
>made
>+ * using credentials such as cookies or authorization headers
>+ *
>+ * @productversion FlexJS 0.8
>+ */
>+ COMPILE::JS
>+ public function set withCredentials(value:Boolean):void {
>+ var element:XMLHttpRequest = this.element as XMLHttpRequest;
>+ element.withCredentials = value;
>+ }
>+
> COMPILE::SWF
> private var urlLoader:flash.net.URLLoader;
>
>@@ -606,6 +618,7 @@ package org.apache.flex.net
> }
> }
>
>+
> if (_method !== HTTPConstants.GET &&
> !sawContentType && contentData) {
> element.setRequestHeader(
>
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security. Allow auth credentials to be passed when using cross site calls. This is required as well as setting the Access-Control-Allow-Origin header on the server.
Posted by Harbs <ha...@gmail.com>.
I think Peter wrote the original implementation, and you are probably right on most of your points that a lot of the code should be pulled out into beads.
I’ll let Peter or Alex comment on the original implementation, but it could be it’s time to re-write some of it.
Harbs
> On Apr 9, 2017, at 10:14 AM, Justin Mclean <ju...@classsoftware.com> wrote:
>
> Hi,
>
>> Therefore code to deal with authentication should not be in the base component.
>
> So in that case should the code dealing with HTTPs headers should be removed?
>
> I can see the class is also dealing with timeouts should that also be removed and placed in a bead?
>
> The class is also missing JS implementations of addBead, getBeadByType and removeBead so it currently doesn’t support beads on the JS side. I assume we will need to add JS implementations of those methods?
>
> The method send deals with AIR only HTTP status events, that seems like it should not be there at all? Why isn’t that a bead?
>
> It only has SWF version of error handlers to deal with various AS errors surely that also should also be a bead?
>
> It seem to me we’re being rather inconsistent here.
>
> Thanks,
> Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Alex Harui <ah...@adobe.com>.
Another way to think about it: It would be awesome if, as a customer used
a code coverage tool to test their app, that the code coverage of the
framework code in their app reached 100% as the customer's code coverage
reached 100%. Such a thing is generally never going to happen for regular
Flex apps.
-Alex
On 4/9/17, 11:14 PM, "Alex Harui" <ah...@adobe.com> wrote:
>At the highest level, the reason for beads and PAYG is not just for SWF
>code, it is for JS code as well. And the net result should be that code
>you didn't in your app, isn't in your app. Lots of people will need CORS
>support, but lots of people won't. Now we could go overboard putting
>every method in its own bead, but that won't help the folks who aren't
>using some feature as the overhead would overwhelm the advantages of not
>carrying around the code of a feature that isn't used. So when you break
>stuff out into a bead, we want the net code size and performance for
>someone not using the bead to be the same or better.
>
>Harbs is right that this is a completely different way of thinking about
>code than for the regular Flex SDK. The philosophy of "it's only a little
>bit of extra code" is why the regular Flex SDK's UIComponent is 13,000
>lines long. And why I had several unhappy customers bring their Flex app
>to me looking to save size and performance and I had to tell them that the
>cost of refactoring UIComponent was too high and they were out of luck.
>
>It is certainly worth revisiting the code we've written to see if other
>stuff can be pulled out into beads, so thanks for reviewing HTTPService.
>It is one of the early classes and probably needs a review. Given the
>above, my responses are inline below.
>
>On 4/9/17, 7:14 AM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>
>>Hi,
>>
>>> Therefore code to deal with authentication should not be in the base
>>>component.
>>
>>So in that case should the code dealing with HTTPs headers should be
>>removed?
>
>Possibly. As long as the implementation for folks not using headers would
>be smaller/faster.
>
>>
>>I can see the class is also dealing with timeouts should that also be
>>removed and placed in a bead?
>
>Possibly, but I claim that nobody should go into production without
>supporting timeouts, so I would leave that baked in.
>
>>
>>The class is also missing JS implementations of addBead, getBeadByType
>>and removeBead so it currently doesn’t support beads on the JS side. I
>>assume we will need to add JS implementations of those methods?
>
>Yes, if they are actually missing. It looks like HTTPService is an
>Istrand, so I'm surprised about that.
>
>>
>>The method send deals with AIR only HTTP status events, that seems like
>>it should not be there at all? Why isn’t that a bead?
>
>Again, if you can find a way to refactor that out without making the
>HTTPService bigger/slower, then great.
>
>>
>>It only has SWF version of error handlers to deal with various AS errors
>>surely that also should also be a bead?
>
>Like Timeout, I claim that nobody should go into production without
>handling errors. So probably, the JS errors need to be handled right in
>HTTPService or HTTPServiceBase. Maybe there should be a common set of
>events generated.
>
>Thanks,
>-Alex
>
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Alex Harui <ah...@adobe.com>.
On 4/17/17, 2:44 AM, "piotrz" <pi...@gmail.com> wrote:
>Yes I was thinking about remove, but it look like you really need it and
>need
>to rely on events, so ok I don't want to argue.
>
>But maybe let's creat another one who just switch On property without rely
>on any events as it was suggested above.
We can have multiple version of beads some of which are very simple and
some more elaborate. We want to give our users choices. So I don't agree
that we should push that work into the application code. It is fine for
Justin to provide two versions of handling withCredentials, and volunteers
are welcome to provide others.
My 2 cents,
-Alex
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by piotrz <pi...@gmail.com>.
Yes I was thinking about remove, but it look like you really need it and need
to rely on events, so ok I don't want to argue.
But maybe let's creat another one who just switch On property without rely
on any events as it was suggested above.
Piotr
-----
Apache Flex PMC
piotrzarzycki21@gmail.com
--
View this message in context: http://apache-flex-development.2333347.n4.nabble.com/Re-git-commit-flex-asjs-refs-heads-develop-CORS-security-Allow-auth-credentials-to-be-passed-when-us-tp61050p61150.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> So if we have some problems here - maybe you should remove this bead from
> framework, but since you need it - put it in your end application.
IMO Other people are very likely to need this at some point so it’s needed in the framework. Are you suggesting we go back to a non-bead implementation and just use the two line function instead?
> In framework itself add simple bead as Alex suggested.
If you look at the bead's code it's is actually reasonably simple. It's about the same in scape and size and functionality as say the MDL disable bead, i.e. a set strand method, a single getter and setter pair and a couple of small event handlers.
> If I correct understand HttpService can be both with credential and not?
It's not supporting user credentials the bead doesn’t deal with that. The bead just sets a single property withCredentials to enable/disable cross domain XMLHttpRequest requests.
On the AS side you would use a cross domain policy, but on JS you need to set withCredentials on the client side as well.
Thanks,
Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by piotrz <pi...@gmail.com>.
Hi Justin,
So if we have some problems here - maybe you should remove this bead from
framework, but since you need it - put it in your end application.
In framework itself add simple bead as Alex suggested. Let's go with
providing simple beads by framework leaving to the user more sophisticated
scenario.
If I correct understand HttpService can be both with credential and not? -
That's something which should be cover by the user.
Piotr
-----
Apache Flex PMC
piotrzarzycki21@gmail.com
--
View this message in context: http://apache-flex-development.2333347.n4.nabble.com/Re-git-commit-flex-asjs-refs-heads-develop-CORS-security-Allow-auth-credentials-to-be-passed-when-us-tp61050p61147.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> I don't know much about CORS. If you make a login call in order to
> authenticate the user and the server you call with the login is on a
> different domain, how do you get the transaction to happen if
> withCredentials are false?
The server sets an Access-Control-Allow-Origin header to allow cross domain calls. This by default this doesn’t pass credentials / cookies. In the login case the user credentials are likely passed in a URL query string (i.e. as form variable). Think of your classic login name and password form. It's only future requests that require the users authorisation cookie(s) to be passed on and that requires withCredentials set to true.
Thanks,
Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Alex Harui <ah...@adobe.com>.
On 4/16/17, 11:00 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>> So, good first try at a bead. Maybe we don't agree on the scenario.
>>It
>> looked like this bead was considering a scenario where on one call, you
>> might set withCredentials=true, and on a subsequent call, you might set
>> withCredentials=false,
>
>Yes I said that was a requirement in one of my emails. It also why the
>function accepted it as a parameter rather than just setting it to true.
>
>> in which case you don't want to remove the listeners in postSend.
>
>AFAIKS you do want to remove them, otherwise if you called send again
>they are replaced and there’s a potential for a memory leak.
If you remove the preSend listener in the postSend, how will your logic
ever see the next send? IMO you have to leave the listeners on and do any
cleanup in the strand setter. But again, I don't think most beads need to
worry about being removed. The major use of beads is for compile-time and
startup-time configuration, not runtime configuration after startup.
>
>> So, by encapsulating this code in a bead, a simpler version can be
>>used for the
>> "set it once" crowd, and this more elaborate version could be used by
>> someone who does want to change withCredentials in different calls.
>
>So I should create an extra bead CORSWithCredentialsIsTrueBead where is
>it always true? Where user authentication is used there is probably
>always going to be a least one call i.e. the login call before the user
>is authenticated that you don’t want to pass the credentials on. I would
>guess that having the ability to set it would be the more common use case
>so this extra bead is probably not needed.
I don't know much about CORS. If you make a login call in order to
authenticate the user and the server you call with the login is on a
different domain, how do you get the transaction to happen if
withCredentials are false?
I would add the very simple CORSWithCredentialsIsTrueBead because we want
to give our users choices and there are plenty of ways to get the login
that don't require the more elaborate bead you have proposed, such has
having a separate HTTPService instance for the login and another one for
other transactions.
-Alex
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> So, good first try at a bead. Maybe we don't agree on the scenario. It
> looked like this bead was considering a scenario where on one call, you
> might set withCredentials=true, and on a subsequent call, you might set
> withCredentials=false,
Yes I said that was a requirement in one of my emails. It also why the function accepted it as a parameter rather than just setting it to true.
> in which case you don't want to remove the listeners in postSend.
AFAIKS you do want to remove them, otherwise if you called send again they are replaced and there’s a potential for a memory leak.
> So, by encapsulating this code in a bead, a simpler version can be used for the
> "set it once" crowd, and this more elaborate version could be used by
> someone who does want to change withCredentials in different calls.
So I should create an extra bead CORSWithCredentialsIsTrueBead where is it always true? Where user authentication is used there is probably always going to be a least one call i.e. the login call before the user is authenticated that you don’t want to pass the credentials on. I would guess that having the ability to set it would be the more common use case so this extra bead is probably not needed.
> Regarding removing listeners, I just noticed that
> ElementWrapper.removeBead does not set the beads strand to null.
This seems like a bug to me, it should be doing that. That's part of the reason I added the remove listeners in the post send handler.
> Right now I think it should. What do others think? And then if you want to
> worry about being removed you could check for null and remove listeners,
> but IMO, there should be very few if any scenarios where beads get removed.
While not too common I've run into a couple of times. I just run into another earlier today with date formatting.
Thanks,
Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by ju...@classsoftware.com.
Hi,
I’ve looked through the code again and other than a couple of minor changes I can't see any obvious way to to either reduce the lines of code or the logic error Alex is referring to.
Someone care to enlighten me?
Thanks,
Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> I looked at your code. There seems to be some logic issues and your
> version is probably the most elaborate.
Elaborate how? It contains the minimum required to get it to work that give the same functionality as that original line method.
Re logic issue iI assume you mean it will only set the headers for a single call? However I saw no other way of being able to clean up the event listeners other than adding yep more code to HTTPService which you would of likely complained about.
Thanks,
Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Alex Harui <ah...@adobe.com>.
Holiday in the US today. I'll reply in detail later. A few quick
comments in-line...
On 4/15/17, 10:53 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>So just for giggles I implemented this as has recommended.
>
>The PAYG cost ends up being the following if you use it:
>- The loading cost for JS is an extra file in debug / 50+ lines of
>compiled code. Runtime cost in JS is two events dispatched and 50+ extra
>lines of code needed to be run.
I looked at your code. There seems to be some logic issues and your
version is probably the most elaborate. Simpler ones should have lower
cost. Take another look and see if you can see it. Maybe others can see
it as well.
>- 4 extra lines of code in AS version. The run time cost for AS is two
>events dispatched.
IMO, this cost is amortized and the final net cost for both platforms will
be <= 0. I'll explain more later.
-Alex
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> Regarding removing listeners, I just noticed that
> ElementWrapper.removeBead does not set the beads strand to null.
Once this is fixed I’ll no longer need the postSend handler and I’ll change set strand to:
public function set strand(value:IStrand):void {
_strand = value;
if (_strand == null) {
IEventDispatcher(_strand).removeEventListener("preSend", preSendHandler);
}
else {
IEventDispatcher(_strand).addEventListener("preSend", preSendHandler);
}
}
Thanks,
Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Alex Harui <ah...@adobe.com>.
OK, more detail as promised.
So, good first try at a bead. Maybe we don't agree on the scenario. It
looked like this bead was considering a scenario where on one call, you
might set withCredentials=true, and on a subsequent call, you might set
withCredentials=false, in which case you don't want to remove the
listeners in postSend.
I was thinking of a much simpler scenario where you set withCredentials on
the XHR instance once and it stays unchanged. IMO, these two scenarios
makes a good case for beads. Before I saw your more elaborate version it
hadn't occurred to me that someone might want to make certain calls with
withCredentials being true sometimes and false other times. So, by
encapsulating this code in a bead, a simpler version can be used for the
"set it once" crowd, and this more elaborate version could be used by
someone who does want to change withCredentials in different calls.
It occurred to me only this morning that HTTPService is an ElementWrapper
and thus the beads can assume the element is created by the time the bead
is added.
Also, in most simple cases, the adding of the bead replaces the need for a
boolean property. IOW, if you don't need withCredentials set to true, you
don't add the bead and element.withCredentials remains at its default
false value.
So, the simplest version of this bead could look like this:
public function set strand(value:IStrand):void
{
((value as ElementWrapper).element as XHR).withCredentials = true;
}
No need for any event listeners or dispatching of preSend/postSend events.
But, you did point out that custom headers could also be implemented via a
bead. And a custom headers bead might need the preSend event as well.
And Om pointed out that other JS libraries provide interceptor functions
that are similar to a preSend event, so adding a "preSend" event seems
worth the cost. Other beads may leverage this, and thus you can't
attribute its cost to this particular CORS implementation.
And if you do move the custom headers code to a bead, then it will make
several of our examples smaller, so that's why I said that the final net
cost is <=0.
Regarding removing listeners, I just noticed that
ElementWrapper.removeBead does not set the beads strand to null. Right
now I think it should. What do others think? And then if you want to
worry about being removed you could check for null and remove listeners,
but IMO, there should be very few if any scenarios where beads get removed.
IMO, there is "application developer decision making" and "framework
developer decision making". When developing an application it is fine to
say "this is the way I need it to work" and not worry too much about
re-usability of code. But we really need the developers of the framework
to consider how other folks may need to use or extend the framework code
and provide events/hooks for that so the framework is "Flex-ible". So a
"preSend" event should be good. Is a "postSend" ever going to be needed?
Possibly, I haven't thought it through.
Finally, users of optional features will pay a higher cost than if we just
in-line everything. But then they can remove code from other options they
did not use. And, as I mentioned earlier, it is fine to produce a
separate component with everything baked in if it turns out that folks
need that kind of optimization. But realize that EVERY Flex app ever
built would be smaller and faster if if didn't use Flex. IMO, no
framework exists to make your code small and fast, it exists to improve
developer productivity. And while Flex made it really productive to build
the app, IMO, it failed for really big enterprise apps in the "last mile"
where all of that optional code did become a factor in size and
performance. There's a good chance that nobody on this mailing list ever
hit this problem because those that did probably aren't Flex customers any
more, but I saw it many times, from companies that have the potential to
hire many of us and drive the next generation of Flex. FlexJS is being
designed differently so that the "first mile" is as close to as fast as it
is in regular Flex, but also so that the "last mile" isn't where you run
out of resources. It does take a different mindset from regular Flex, but
I believe, and I think the rest of the PMC believes, that it is the right
direction for us to take.
My 2 cents,
-Alex
On 4/15/17, 10:53 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>So just for giggles I implemented this as has recommended.
>
>The PAYG cost ends up being the following if you use it:
>- The loading cost for JS is an extra file in debug / 50+ lines of
>compiled code. Runtime cost in JS is two events dispatched and 50+ extra
>lines of code needed to be run.
>- 4 extra lines of code in AS version. The run time cost for AS is two
>events dispatched.
>
>The PAYG cost ends up being the following if you don’t use it for both AS
>and JS:
>- Two events dispatched and a couple of extra lines of code.
>
>If we were to go with my original function implementation the PAYG cost
>for AS is zero and the PAYG cost for JS is a 2 line function that is only
>used if called.
>
>There also seem a problem with JS only beads in that I couldn’t work out
>how to add it to the manifest as a JS only class. Anyone have any idea on
>how to do that? Adding it to NetworkClasses.as in a COMPILE::JS block
>didn’t work and caused complication errors.
>
>Thanks,
>Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
So just for giggles I implemented this as has recommended.
The PAYG cost ends up being the following if you use it:
- The loading cost for JS is an extra file in debug / 50+ lines of compiled code. Runtime cost in JS is two events dispatched and 50+ extra lines of code needed to be run.
- 4 extra lines of code in AS version. The run time cost for AS is two events dispatched.
The PAYG cost ends up being the following if you don’t use it for both AS and JS:
- Two events dispatched and a couple of extra lines of code.
If we were to go with my original function implementation the PAYG cost for AS is zero and the PAYG cost for JS is a 2 line function that is only used if called.
There also seem a problem with JS only beads in that I couldn’t work out how to add it to the manifest as a JS only class. Anyone have any idea on how to do that? Adding it to NetworkClasses.as in a COMPILE::JS block didn’t work and caused complication errors.
Thanks,
Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Alex Harui <ah...@adobe.com>.
On 4/11/17, 9:44 PM, "omuppi1@gmail.com on behalf of OmPrakash Muppirala"
<omuppi1@gmail.com on behalf of bigosmallm@gmail.com> wrote:
>
>One common approach I have seen in JS webservice libraries is to allow the
>users to attach an interceptor function which will carry the requrst
>object
>in the function args. The user can then attach any headers they want to
>the request object inside the function.
>
>This would be a static method on HTTPService. That way, for example, once
>authenticated, all api calls could be intercepted and authentication
>headers attached. Or perhaps CORS headers attached if the situation
>requires.
>
>In runtime we check
>
>if(HttpService.interceptor) {
> HTTPService. interceptor.call(this.request);
>}
If I understand that pattern, it and other callback patterns are ok, but
not my favorite pattern because it sort of implies that there is only one
interceptor. That's what I like about events and other multi-subscriber
patterns: any number of other objects can find out what is going on and
remove themselves from the list of folks that care.
My 2 cents,
-Alex
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the server.
Posted by OmPrakash Muppirala <bi...@gmail.com>.
On Apr 10, 2017 11:40 PM, "Alex Harui" <ah...@adobe.com> wrote:
IMO, a bead is still best, but your proposed implementation isn't what I
would recommend. With beads, we are trying for loose coupling, separation
of concerns, abstraction, encapsulation, and a bunch of other Computer
Science practices. Hopefully, any communication between the strand and
bead is general and not specific to certain beads. If your strand code
actually knows the type of the bead, then your code is probably not using
abstraction properly.
One pattern I like, that we've used in several components, is lifecycle
events. So, the send() method might dispatch an event called
"prepareToSend". That is only one line of new code. Your CORS bead, and
anybody else's beads that add capabilities to HTTPService would simply
listen for that event, grab the element and fiddle with the element as
needed. Other beads might add headers, scan the URL for exploits, etc.
HTH,
-Alex
One common approach I have seen in JS webservice libraries is to allow the
users to attach an interceptor function which will carry the requrst object
in the function args. The user can then attach any headers they want to
the request object inside the function.
This would be a static method on HTTPService. That way, for example, once
authenticated, all api calls could be intercepted and authentication
headers attached. Or perhaps CORS headers attached if the situation
requires.
In runtime we check
if(HttpService.interceptor) {
HTTPService. interceptor.call(this.request);
}
Thanks,
Om
On 4/10/17, 6:56 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>I’ll respond to your email in full later.
>
>Lets assume we make it a bead.
>
>In this case it will add more lines of code (about a dozen) to
>HTTPService and will have a bigger cost people who both use it and don’t
>use it. Currently it’s only a two line method and there no runtime cost
>to people who don’t use it.
>
>That doesn’t seem PAYG to me. Do you still think that is best way
>forward? Or can you suggest another way to get this to work that is no
>cost to people who don’t use it?
>
>Here's what I think would need to be done. I would need to add something
>like this to the JS send method (or call new method(s) that do something
>similar).
>
>int noBeads:int = 0;
>for (int i = 0; i < noBeads; i++) {
> if (beads[i] is CORSAuthenicationBead) {
> element.withAuthenication = (beads[i] as
>CORSAuthenicationBead).withAuthenication;
> }
>}
>
>(withAuthenication needs to set at runtime not just when the bead is
>added)
>
>We could perhaps create a CORSAuthenicationBead and a
>CORSRuntimeAuthenicationBead, but that’s will add more code/cost, as well
>as the code above we would also need to add code in addBead as well to
>set withAuthenication.
>
> But wait addBead only exist on the AS side not the JS side (unless sI’m
>missing something):
>COMPILE::SWF
>public function addBead(bead:IBead):void
>
>And this it looks like the set strand method is broken on JS:
> public function set strand(value:IStrand):void
> {
> _strand = value;
> if (_beads == null)
> {
> for each (var bead:IBead in beads)
> addBead(bead);
> }
>
> dispatchEvent(new org.apache.flex.events.Event("beadsAdded"));
>}
>
>So we also would need add a JS version of addBead as well with again more
>code and cost.
>
>So I’ll ask again given the above do you think that a bead is best here?
>Especially when it adds runtime cost (runtime and size) to user if they
>use it or not?
>
>Thanks,
>Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Alex Harui <ah...@adobe.com>.
IMO, a bead is still best, but your proposed implementation isn't what I
would recommend. With beads, we are trying for loose coupling, separation
of concerns, abstraction, encapsulation, and a bunch of other Computer
Science practices. Hopefully, any communication between the strand and
bead is general and not specific to certain beads. If your strand code
actually knows the type of the bead, then your code is probably not using
abstraction properly.
One pattern I like, that we've used in several components, is lifecycle
events. So, the send() method might dispatch an event called
"prepareToSend". That is only one line of new code. Your CORS bead, and
anybody else's beads that add capabilities to HTTPService would simply
listen for that event, grab the element and fiddle with the element as
needed. Other beads might add headers, scan the URL for exploits, etc.
HTH,
-Alex
On 4/10/17, 6:56 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>I’ll respond to your email in full later.
>
>Lets assume we make it a bead.
>
>In this case it will add more lines of code (about a dozen) to
>HTTPService and will have a bigger cost people who both use it and don’t
>use it. Currently it’s only a two line method and there no runtime cost
>to people who don’t use it.
>
>That doesn’t seem PAYG to me. Do you still think that is best way
>forward? Or can you suggest another way to get this to work that is no
>cost to people who don’t use it?
>
>Here's what I think would need to be done. I would need to add something
>like this to the JS send method (or call new method(s) that do something
>similar).
>
>int noBeads:int = 0;
>for (int i = 0; i < noBeads; i++) {
> if (beads[i] is CORSAuthenicationBead) {
> element.withAuthenication = (beads[i] as
>CORSAuthenicationBead).withAuthenication;
> }
>}
>
>(withAuthenication needs to set at runtime not just when the bead is
>added)
>
>We could perhaps create a CORSAuthenicationBead and a
>CORSRuntimeAuthenicationBead, but that’s will add more code/cost, as well
>as the code above we would also need to add code in addBead as well to
>set withAuthenication.
>
> But wait addBead only exist on the AS side not the JS side (unless sI’m
>missing something):
>COMPILE::SWF
>public function addBead(bead:IBead):void
>
>And this it looks like the set strand method is broken on JS:
> public function set strand(value:IStrand):void
> {
> _strand = value;
> if (_beads == null)
> {
> for each (var bead:IBead in beads)
> addBead(bead);
> }
>
> dispatchEvent(new org.apache.flex.events.Event("beadsAdded"));
>}
>
>So we also would need add a JS version of addBead as well with again more
>code and cost.
>
>So I’ll ask again given the above do you think that a bead is best here?
>Especially when it adds runtime cost (runtime and size) to user if they
>use it or not?
>
>Thanks,
>Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
I’ll respond to your email in full later.
Lets assume we make it a bead.
In this case it will add more lines of code (about a dozen) to HTTPService and will have a bigger cost people who both use it and don’t use it. Currently it’s only a two line method and there no runtime cost to people who don’t use it.
That doesn’t seem PAYG to me. Do you still think that is best way forward? Or can you suggest another way to get this to work that is no cost to people who don’t use it?
Here's what I think would need to be done. I would need to add something like this to the JS send method (or call new method(s) that do something similar).
int noBeads:int = 0;
for (int i = 0; i < noBeads; i++) {
if (beads[i] is CORSAuthenicationBead) {
element.withAuthenication = (beads[i] as CORSAuthenicationBead).withAuthenication;
}
}
(withAuthenication needs to set at runtime not just when the bead is added)
We could perhaps create a CORSAuthenicationBead and a CORSRuntimeAuthenicationBead, but that’s will add more code/cost, as well as the code above we would also need to add code in addBead as well to set withAuthenication.
But wait addBead only exist on the AS side not the JS side (unless sI’m missing something):
COMPILE::SWF
public function addBead(bead:IBead):void
And this it looks like the set strand method is broken on JS:
public function set strand(value:IStrand):void
{
_strand = value;
if (_beads == null)
{
for each (var bead:IBead in beads)
addBead(bead);
}
dispatchEvent(new org.apache.flex.events.Event("beadsAdded"));
}
So we also would need add a JS version of addBead as well with again more code and cost.
So I’ll ask again given the above do you think that a bead is best here? Especially when it adds runtime cost (runtime and size) to user if they use it or not?
Thanks,
Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Alex Harui <ah...@adobe.com>.
Responding to you and Justin in this post...
On 4/10/17, 1:55 AM, "Greg Dove" <gr...@gmail.com> wrote:
>Actually I wasn't sure whether the compiler eliminated the dead js code.
>But we know that it can.
>
>I get the point about the 'swiss-army-knife' but I don't think that
>applies
>here, because I think this is more a 'standard tool'. I can only directly
>recall one Flex project in the last 6 years that did not require the swf
>crossdomain security model. Perhaps I have forgotten some, but I suspect
>that the world is becoming more interconnected, not less. I know there
>will
>definitely be some who don't need this, and I guess there are certain
>types
>of application that always will be single-domain. But unless I
>misunderstand, it sounds like this need might be more for the majority
>than
>the minority.
>
>The point about the GCC stuff though I think is a little contrasting to
>the
>PAYG philosphy elsewhere.
Because AS allows you to write:
myHttpService["withCredentials"] = true;
Our tools currently add @export to all public APIs. And thus, the code
Justin added is not eliminated. I just compiled FlexJSStore and the
string "withCredentials" is in the optimized JS twice.
Certainly, the tools could become smarter, but they aren't now, and we
have a pattern and protocol for adding features, which is to use beads.
And even then, we want the framework to scale to supporting really large
enterprise apps where not all files are compiled at the same time and APIs
are referenced and shared across these compilation sets. And I don't
really think it is practical to create a tool chain that will ever figure
out what is dead code across compilation sets. I believe the bead pattern
will be great for optimizing in these development environments.
>
>"AFAIK, public APIs have @export and are kept around. Which will be a
>good thing
>in multi-module apps some day."
>
>It sounds like we are keeping a lot of code that could be optimized or
>automatically excluded because it will be a good thing "some day" (the day
>when we support module loading). That sounds like the opposite of PAYG to
>me, because many projects may not need 'module loading' (and we don't have
>this yet anyway). I'm just trying to throw a potentially different way of
>looking at this in here, because, in theory at least, the closure compiler
>should be able to make a whole bunch of stuff go away in the release build
>if it is not used (dead code elimination, DCE). In practice, I have not
>tried it with any sizeable codebase, but I know other typedLanguage-toJS
>compilers get the job done here, and I know that things like including
>reflection data or not are also choices included in the options for output
>in at least one other toJS compiler.
>
>If this can work properly then we could actually have our cake and eat it
>too. And yes the future might mean other things for modules, but perhaps
>there are ways to get that working even with the renaming (DCE maybe not
>so
>much, I agree).
>
IMO, we would provide different compiler options for those who can compile
everything at once. I think what would be required at the simplest level
is some promise from the developer that they did not use any bracket
access (myService["withCredentials"]) in their code, and I suppose that
would mean we couldn't use bracket access in the framework code either
(which I hope we don't do much of, if any. And maybe we could get smarter
and find bracket access and inform GCC which APIs to void renaming, but
then you couldn't do "string math" in bracket access or use string
variables without code-flow analysis in the compiler.
myService["with" + "Credentials"]
var foo:String = "withCredentials";
myService[foo];
But again, those doing really big apps where they don't compile everything
at once will need some other way to optimize and being able to choose what
beads they want to composite is the best plan I can think of so far.
>On Mon, Apr 10, 2017 at 8:32 PM, Justin Mclean <ju...@classsoftware.com>
>wrote:
>
>> Hi,
>>
>> > And the net result should be that code you didn't in your app, isn't
>>in
>> your app.
>>
>> As Greg pointed out the compiler can removed unused JS code i.e. methods
>> that are not called. See for instance [1] for details. So in this case
>>if
>> you don’t use it will not end up in the AS code (as it’s JS only) and
>>will
>> not end up in production JS code.
This is not currently true, and probably can't be true for some
development environments. See my response to Greg above.
>>
>> > So when you break stuff out into a bead, we want the net code size and
>> performance for
>> > someone not using the bead to be the same or better.
>>
>> In this case it is the same, if it was moved to a bead it would be worse
>> for people who needed it. Only by tiny amounts but that’s the point
>>you’re
>> trying to make i.e. slow dead by a 1000 tiny additions, and as you say
>>lots
>> of people will need CORS support.
Yes. That is what Pay-as-you-go (PAYG) means.
>>
>> > It is certainly worth revisiting the code we've written to see if
>>other
>> > stuff can be pulled out into beads, so thanks for reviewing
>>HTTPService.
>> > It is one of the early classes and probably needs a review. Given the
>> > above, my responses are inline below.
>>
>> We should provide clear documentation on this, currently there is none?
>> Having our own code not following the same philosophy is a barrier for
>> people contributing. In most other Apache projects if someone submits a
>> patch that may not be “perfect” other committers will help to improve
>>it.
>> Pull request welcome and all that.
I saw you do your "Intro to Flex" at ApacheCon Portland. It was well
done. You might be a good candidate to produce introductory learning
content for FlexJS. So maybe if we can get you to understand the value of
beads and PAYG you can help us produce this documentation.
Again, this is a new effort. It isn't regular Flex where we had a whole
team of developers and testers and doc writers. This is just a small team
of volunteers. There are no paved roads with signs. You have to follow
the wagon wheel ruts and figure stuff out. We need more volunteers to
help pave the roads and post the signs.
>>
>> > Possibly. As long as the implementation for folks not using headers
>> would
>> > be smaller/faster.
>>
>> That is likely to be the case, but it will be slower for people who do
>>use
>> headers. How do we determine what is the most common use case? Isn’t
>>that
>> going to vary by use / customer so we going to end penalising someone no
>> matter which way we go.
I'm sure I've written this in past emails, but basically, PAYG is all
about it being slower for those who use more code. But beads and
composition also attempts to avoid the entire "which is more common"
debate. At least in hardware stores in the US, when I need to buy a
screwdriver, I am confronted with dozens of choices each with slightly
different set of features. Thank goodness there isn't only one
screwdriver with the replaceable tips. Because that one doesn't work in a
few cramped places in my house. I have a small one for that.
So, if you put your CORS code in a bead, folks who need it but are using
the Basic HTTPService will add it to their HTTPService. And the
HTTPService in Express (or maybe a new ExpressJS set that has code without
SWF equivalents) will have your CORS bead baked in. And if folks complain
that it the bead overhead is too high, then you can inline some of the
beads and offer that as well.
FlexJS just wants to offer choices to people. Old Flex was trying to make
everyone use that one screwdriver. It didn't work well for enough folks.
I don't want us to make that mistake again. Old Flex, we wasted time
debating what the "right way" was. We can avoid that debate as well.
Just provide different ways. The customers will decide.
>>
>> > Possibly, but I claim that nobody should go into production without
>> > supporting timeouts, so I would leave that baked in.
>>
>> So why does this not apply to the method I added? Every single project
>> I’ve worked on has made cross domain calls. It's very uncommon for web
>> applications to have their database, rest services, API calls on the
>>same
>> domain and quite common to call 3rd party services.
Very uncommon means you agree it isn't everybody. So we give folks
choices.
>>
>> And I’m sure no one would go into production without supporting security
>> of some form. I’m concerned her that we are making security an optional
>> feature here and not having it supported by default.
If you aren't going across domain, you don't need CORS, AFAIK.
>>
>> > Again, if you can find a way to refactor that out without making the
>> > HTTPService bigger/slower, then great.
>>
>> Anytime you place something in a bead it is going to be bigger and
>>slower
>> for the people that need that functionality. It will be faster/smaller
>>for
>> the others who don't. So again how do we determine what is the most
>>common
>> use case? Do more people use AIR status code than don’t use them? I
>>would
>> guess not in this case and it should be a bead but it’s just a guess.
>>
>> > Like Timeout, I claim that nobody should go into production without
>> > handling errors. So probably, the JS errors need to be handled right
>>in
>> > HTTPService or HTTPServiceBase.
>>
>> So you're saying that HTTPService is where JS security errors should be
>> handled and not in a bead?
That's where I'd put it. Others may have a different opinion.
Thanks,
-Alex
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the server.
Posted by Greg Dove <gr...@gmail.com>.
Actually I wasn't sure whether the compiler eliminated the dead js code.
But we know that it can.
I get the point about the 'swiss-army-knife' but I don't think that applies
here, because I think this is more a 'standard tool'. I can only directly
recall one Flex project in the last 6 years that did not require the swf
crossdomain security model. Perhaps I have forgotten some, but I suspect
that the world is becoming more interconnected, not less. I know there will
definitely be some who don't need this, and I guess there are certain types
of application that always will be single-domain. But unless I
misunderstand, it sounds like this need might be more for the majority than
the minority.
The point about the GCC stuff though I think is a little contrasting to the
PAYG philosphy elsewhere.
"AFAIK, public APIs have @export and are kept around. Which will be a
good thing
in multi-module apps some day."
It sounds like we are keeping a lot of code that could be optimized or
automatically excluded because it will be a good thing "some day" (the day
when we support module loading). That sounds like the opposite of PAYG to
me, because many projects may not need 'module loading' (and we don't have
this yet anyway). I'm just trying to throw a potentially different way of
looking at this in here, because, in theory at least, the closure compiler
should be able to make a whole bunch of stuff go away in the release build
if it is not used (dead code elimination, DCE). In practice, I have not
tried it with any sizeable codebase, but I know other typedLanguage-toJS
compilers get the job done here, and I know that things like including
reflection data or not are also choices included in the options for output
in at least one other toJS compiler.
If this can work properly then we could actually have our cake and eat it
too. And yes the future might mean other things for modules, but perhaps
there are ways to get that working even with the renaming (DCE maybe not so
much, I agree).
On Mon, Apr 10, 2017 at 8:32 PM, Justin Mclean <ju...@classsoftware.com>
wrote:
> Hi,
>
> > And the net result should be that code you didn't in your app, isn't in
> your app.
>
> As Greg pointed out the compiler can removed unused JS code i.e. methods
> that are not called. See for instance [1] for details. So in this case if
> you don’t use it will not end up in the AS code (as it’s JS only) and will
> not end up in production JS code.
>
> > So when you break stuff out into a bead, we want the net code size and
> performance for
> > someone not using the bead to be the same or better.
>
> In this case it is the same, if it was moved to a bead it would be worse
> for people who needed it. Only by tiny amounts but that’s the point you’re
> trying to make i.e. slow dead by a 1000 tiny additions, and as you say lots
> of people will need CORS support.
>
> > It is certainly worth revisiting the code we've written to see if other
> > stuff can be pulled out into beads, so thanks for reviewing HTTPService.
> > It is one of the early classes and probably needs a review. Given the
> > above, my responses are inline below.
>
> We should provide clear documentation on this, currently there is none?
> Having our own code not following the same philosophy is a barrier for
> people contributing. In most other Apache projects if someone submits a
> patch that may not be “perfect” other committers will help to improve it.
> Pull request welcome and all that.
>
> > Possibly. As long as the implementation for folks not using headers
> would
> > be smaller/faster.
>
> That is likely to be the case, but it will be slower for people who do use
> headers. How do we determine what is the most common use case? Isn’t that
> going to vary by use / customer so we going to end penalising someone no
> matter which way we go.
>
> > Possibly, but I claim that nobody should go into production without
> > supporting timeouts, so I would leave that baked in.
>
> So why does this not apply to the method I added? Every single project
> I’ve worked on has made cross domain calls. It's very uncommon for web
> applications to have their database, rest services, API calls on the same
> domain and quite common to call 3rd party services.
>
> And I’m sure no one would go into production without supporting security
> of some form. I’m concerned her that we are making security an optional
> feature here and not having it supported by default.
>
> > Again, if you can find a way to refactor that out without making the
> > HTTPService bigger/slower, then great.
>
> Anytime you place something in a bead it is going to be bigger and slower
> for the people that need that functionality. It will be faster/smaller for
> the others who don't. So again how do we determine what is the most common
> use case? Do more people use AIR status code than don’t use them? I would
> guess not in this case and it should be a bead but it’s just a guess.
>
> > Like Timeout, I claim that nobody should go into production without
> > handling errors. So probably, the JS errors need to be handled right in
> > HTTPService or HTTPServiceBase.
>
> So you're saying that HTTPService is where JS security errors should be
> handled and not in a bead?
>
> Thanks,
> Justin
>
> 1. https://developers.google.com/closure/compiler/docs/api-tutorial3
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> And the net result should be that code you didn't in your app, isn't in your app.
As Greg pointed out the compiler can removed unused JS code i.e. methods that are not called. See for instance [1] for details. So in this case if you don’t use it will not end up in the AS code (as it’s JS only) and will not end up in production JS code.
> So when you break stuff out into a bead, we want the net code size and performance for
> someone not using the bead to be the same or better.
In this case it is the same, if it was moved to a bead it would be worse for people who needed it. Only by tiny amounts but that’s the point you’re trying to make i.e. slow dead by a 1000 tiny additions, and as you say lots of people will need CORS support.
> It is certainly worth revisiting the code we've written to see if other
> stuff can be pulled out into beads, so thanks for reviewing HTTPService.
> It is one of the early classes and probably needs a review. Given the
> above, my responses are inline below.
We should provide clear documentation on this, currently there is none? Having our own code not following the same philosophy is a barrier for people contributing. In most other Apache projects if someone submits a patch that may not be “perfect” other committers will help to improve it. Pull request welcome and all that.
> Possibly. As long as the implementation for folks not using headers would
> be smaller/faster.
That is likely to be the case, but it will be slower for people who do use headers. How do we determine what is the most common use case? Isn’t that going to vary by use / customer so we going to end penalising someone no matter which way we go.
> Possibly, but I claim that nobody should go into production without
> supporting timeouts, so I would leave that baked in.
So why does this not apply to the method I added? Every single project I’ve worked on has made cross domain calls. It's very uncommon for web applications to have their database, rest services, API calls on the same domain and quite common to call 3rd party services.
And I’m sure no one would go into production without supporting security of some form. I’m concerned her that we are making security an optional feature here and not having it supported by default.
> Again, if you can find a way to refactor that out without making the
> HTTPService bigger/slower, then great.
Anytime you place something in a bead it is going to be bigger and slower for the people that need that functionality. It will be faster/smaller for the others who don't. So again how do we determine what is the most common use case? Do more people use AIR status code than don’t use them? I would guess not in this case and it should be a bead but it’s just a guess.
> Like Timeout, I claim that nobody should go into production without
> handling errors. So probably, the JS errors need to be handled right in
> HTTPService or HTTPServiceBase.
So you're saying that HTTPService is where JS security errors should be handled and not in a bead?
Thanks,
Justin
1. https://developers.google.com/closure/compiler/docs/api-tutorial3
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Alex Harui <ah...@adobe.com>.
At the highest level, the reason for beads and PAYG is not just for SWF
code, it is for JS code as well. And the net result should be that code
you didn't in your app, isn't in your app. Lots of people will need CORS
support, but lots of people won't. Now we could go overboard putting
every method in its own bead, but that won't help the folks who aren't
using some feature as the overhead would overwhelm the advantages of not
carrying around the code of a feature that isn't used. So when you break
stuff out into a bead, we want the net code size and performance for
someone not using the bead to be the same or better.
Harbs is right that this is a completely different way of thinking about
code than for the regular Flex SDK. The philosophy of "it's only a little
bit of extra code" is why the regular Flex SDK's UIComponent is 13,000
lines long. And why I had several unhappy customers bring their Flex app
to me looking to save size and performance and I had to tell them that the
cost of refactoring UIComponent was too high and they were out of luck.
It is certainly worth revisiting the code we've written to see if other
stuff can be pulled out into beads, so thanks for reviewing HTTPService.
It is one of the early classes and probably needs a review. Given the
above, my responses are inline below.
On 4/9/17, 7:14 AM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>> Therefore code to deal with authentication should not be in the base
>>component.
>
>So in that case should the code dealing with HTTPs headers should be
>removed?
Possibly. As long as the implementation for folks not using headers would
be smaller/faster.
>
>I can see the class is also dealing with timeouts should that also be
>removed and placed in a bead?
Possibly, but I claim that nobody should go into production without
supporting timeouts, so I would leave that baked in.
>
>The class is also missing JS implementations of addBead, getBeadByType
>and removeBead so it currently doesn’t support beads on the JS side. I
>assume we will need to add JS implementations of those methods?
Yes, if they are actually missing. It looks like HTTPService is an
Istrand, so I'm surprised about that.
>
>The method send deals with AIR only HTTP status events, that seems like
>it should not be there at all? Why isn’t that a bead?
Again, if you can find a way to refactor that out without making the
HTTPService bigger/slower, then great.
>
>It only has SWF version of error handlers to deal with various AS errors
>surely that also should also be a bead?
Like Timeout, I claim that nobody should go into production without
handling errors. So probably, the JS errors need to be handled right in
HTTPService or HTTPServiceBase. Maybe there should be a common set of
events generated.
Thanks,
-Alex
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> Therefore code to deal with authentication should not be in the base component.
So in that case should the code dealing with HTTPs headers should be removed?
I can see the class is also dealing with timeouts should that also be removed and placed in a bead?
The class is also missing JS implementations of addBead, getBeadByType and removeBead so it currently doesn’t support beads on the JS side. I assume we will need to add JS implementations of those methods?
The method send deals with AIR only HTTP status events, that seems like it should not be there at all? Why isn’t that a bead?
It only has SWF version of error handlers to deal with various AS errors surely that also should also be a bead?
It seem to me we’re being rather inconsistent here.
Thanks,
Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security. Allow auth credentials to be passed when using cross site calls. This is required as well as setting the Access-Control-Allow-Origin header on the server.
Posted by Harbs <ha...@gmail.com>.
On Apr 9, 2017, at 5:54 AM, Justin Mclean <ju...@classsoftware.com> wrote:
>
> Hi,
>
>> But still it would be great to have Bead which holds it.
>
> Can you elaborate why you think it should be in a bead?
Most use cases of HTTPService does not require authentication.
Therefore code to deal with authentication should not be in the base component.
Beads were designed with this exactly this in mind.
> This fixes a security issue on the JS side in an existing component. There no PAYG cost AFAICS and it’s not adding a new feature or something that should be optional.
>
> If user authentication was broken in HTTPService on the AS side but worked in JS would you think the best way to fix that was to add a bead?
Yes. If there is already code in there which deals with authentication, it should be removed and placed in a bead. Not everything in FlexJS is designed 100% PAYG, but the cases where it’s not should be fixed and not compounded upon. It took me a while to get used to this style of coding as well, but I do think that we need the discipline to sticking with as much PAYG as we can.
Fully functional components SHOULD be composed in the Express components.
Thanks,
Harbs
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by piotrz <pi...@gmail.com>.
Understand now. If it not optional than probably can be as you stated as part
of API.
Sorry for misunderstanding.
Piotr
-----
Apache Flex PMC
piotrzarzycki21@gmail.com
--
View this message in context: http://apache-flex-development.2333347.n4.nabble.com/Re-git-commit-flex-asjs-refs-heads-develop-CORS-security-Allow-auth-credentials-to-be-passed-when-us-tp61050p61059.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> But still it would be great to have Bead which holds it.
Can you elaborate why you think it should be in a bead?
This fixes a security issue on the JS side in an existing component. There no PAYG cost AFAICS and it’s not adding a new feature or something that should be optional.
If user authentication was broken in HTTPService on the AS side but worked in JS would you think the best way to fix that was to add a bead?
Thanks,
Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by piotrz <pi...@gmail.com>.
Hi Justin,
But still it would be great to have Bead which holds it. Small separate
extension. Let user used to use bead and think in a FlexJS way not Flex.
Piotr
-----
Apache Flex PMC
piotrzarzycki21@gmail.com
--
View this message in context: http://apache-flex-development.2333347.n4.nabble.com/Re-git-commit-flex-asjs-refs-heads-develop-CORS-security-Allow-auth-credentials-to-be-passed-when-us-tp61050p61057.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
HTTPService on the AS side supports this security feature via cross domain policies built into the Flash Player. CORS is a little more complex and sometimes requires both client and server configuration rather than just mostly server side as AS security does. I think users of FlexJS would want us to support security features for both JS and AS right? Currently JS security in HTTPService is IMO a little broken.
It often said that security is not an optional feature at Apache. It seems you are asking me to make it an optional feature?
Anyway just so other people know what the change was. I added this JS only method to HTTPService:
COMPILE::JS
public function set withCredentials(value:Boolean):void {
var element:XMLHttpRequest = this.element as XMLHttpRequest;
element.withCredentials = value;
}
This line of code is required to support a common scenario i.e. calling a API on a different server from the domain the original page was served from. It’s JS only and has zero performance or size impact on the AS side so as far as I can see there no PAYG implications. Not having it may force a user to disable security to get their application to work. If is not set then cookies and/or user credentials are not passed on with requests which stop most forms of user authentication from working.
I did consider turning it on by default for all requests and adding that one line of code to the send method but they may be some cases where users do not want it.
Thanks,
Justin
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the
server.
Posted by Alex Harui <ah...@adobe.com>.
On 4/8/17, 9:59 PM, "Greg Dove" <gr...@gmail.com> wrote:
>Just a general question about this type of thing:
>
>Because this is JS-only, does GCC eliminate it as deadcode in a release
>version if the method is never used? I would expect that, but I have not
>checked that yet... if it does then perhaps the PAYG concern might be moot
>(love that word!)
AFAIK, public APIs have @export and are kept around. Which will be a good
thing in multi-module apps some day.
Thinking about this some more, it should probably be a bead and then baked
into a more swiss-army-knife version of HTTPService which would be a good
default for the Express package.
-Alex
>
>On Sun, Apr 9, 2017 at 4:48 PM, Alex Harui <ah...@adobe.com> wrote:
>
>> IMO, with PAYG, this would go in an extension of HTTPService. Not all
>> apps will need CORS.
>>
>> Thanks,
>> -Alex
>>
>> On 4/8/17, 8:26 PM, "jmclean@apache.org" <jm...@apache.org> wrote:
>>
>> >Repository: flex-asjs
>> >Updated Branches:
>> > refs/heads/develop 11ef21aae -> 326d69791
>> >
>> >
>> >CORS security. Allow auth credentials to be passed when using cross
>>site
>> >calls. This is required as well as setting the
>> >Access-Control-Allow-Origin header on the server.
>> >
>> >
>> >Project:
>> >https://na01.safelinks.protection.outlook.com/?url=
>> http%3A%2F%2Fgit-wip-us
>> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Frepo&data=02%
>> 7C01%7C%7C58994717190
>> >044f003a908d47ef83ba7%7Cfa7b1b5a7b34438794aed2c178de
>> cee1%7C0%7C0%7C6362730
>> >52005920291&sdata=eQwxowk79ikXeDxbqXV3OeVrXUzTXN
>> fFR0eKzhU8wiw%3D&reserved=
>> >0
>> >Commit:
>> >https://na01.safelinks.protection.outlook.com/?url=
>> http%3A%2F%2Fgit-wip-us
>> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Fcommit%
>> 2F326d6979&data=02%7C01%7C%
>> >7C58994717190044f003a908d47ef83ba7%7Cfa7b1b5a7b34438794aed2c178de
>> cee1%7C0%
>> >7C0%7C636273052005930299&sdata=f8udpkPpLcL1ivRV3LDW0kJARc8QnL
>> hBDHVFGgCko7M
>> >%3D&reserved=0
>> >Tree:
>> >https://na01.safelinks.protection.outlook.com/?url=
>> http%3A%2F%2Fgit-wip-us
>> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Ftree%2F326d6979&
>> data=02%7C01%7C%7C
>> >58994717190044f003a908d47ef83ba7%7Cfa7b1b5a7b34438794aed2c178de
>> cee1%7C0%7C
>> >0%7C636273052005930299&sdata=t6MMifwasbm2bgCuzsVN2q4%
>> 2BCYcB2uB8o7O%2B%2BJu
>> >yZ5w%3D&reserved=0
>> >Diff:
>> >https://na01.safelinks.protection.outlook.com/?url=
>> http%3A%2F%2Fgit-wip-us
>> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Fdiff%2F326d6979&
>> data=02%7C01%7C%7C
>> >58994717190044f003a908d47ef83ba7%7Cfa7b1b5a7b34438794aed2c178de
>> cee1%7C0%7C
>> >0%7C636273052005930299&sdata=gJDrCW4YbwaorNFSXVCVjLyq3lwTC6
>> 7VhCRLEtYlcD0%3
>> >D&reserved=0
>> >
>> >Branch: refs/heads/develop
>> >Commit: 326d69791b37cc2aaac546bcfcd3a51e88716f2f
>> >Parents: 11ef21a
>> >Author: Justin Mclean <jm...@apache.org>
>> >Authored: Sun Apr 9 13:26:30 2017 +1000
>> >Committer: Justin Mclean <jm...@apache.org>
>> >Committed: Sun Apr 9 13:26:30 2017 +1000
>> >
>> >----------------------------------------------------------------------
>> > .../src/main/flex/org/apache/flex/net/HTTPService.as | 13
>> +++++++++++++
>> > 1 file changed, 13 insertions(+)
>> >----------------------------------------------------------------------
>> >
>> >
>> >https://na01.safelinks.protection.outlook.com/?url=
>> http%3A%2F%2Fgit-wip-us
>> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Fblob%2F326d6979%
>> 2Fframeworks%2Fpro
>> >jects%2FNetwork%2Fsrc%2Fmain%2Fflex%2Forg%2Fapache%2Fflex%
>> 2Fnet%2FHTTPServ
>> >ice.as&data=02%7C01%7C%7C58994717190044f003a908d47ef8
>> 3ba7%7Cfa7b1b5a7b3443
>> >8794aed2c178decee1%7C0%7C0%7C636273052005930299&sdata=
>> TcDMCOEVbLKxedpsnACV
>> >OmZon89YgUkvGxOtd%2F3Qky8%3D&reserved=0
>> >----------------------------------------------------------------------
>> >diff --git
>> >a/frameworks/projects/Network/src/main/flex/org/
>> apache/flex/net/HTTPServic
>> >e.as
>> >b/frameworks/projects/Network/src/main/flex/org/
>> apache/flex/net/HTTPServic
>> >e.as
>> >index b939751..3a9968c 100644
>> >---
>> >a/frameworks/projects/Network/src/main/flex/org/
>> apache/flex/net/HTTPServic
>> >e.as
>> >+++
>> >b/frameworks/projects/Network/src/main/flex/org/
>> apache/flex/net/HTTPServic
>> >e.as
>> >@@ -505,6 +505,18 @@ package org.apache.flex.net
>> > return null;
>> > }
>> >
>> >+ /**
>> >+ * Allows Javascript cross-site Access-Control requests to be
>> >made
>> >+ * using credentials such as cookies or authorization headers
>> >+ *
>> >+ * @productversion FlexJS 0.8
>> >+ */
>> >+ COMPILE::JS
>> >+ public function set withCredentials(value:Boolean):void {
>> >+ var element:XMLHttpRequest = this.element as
>>XMLHttpRequest;
>> >+ element.withCredentials = value;
>> >+ }
>> >+
>> > COMPILE::SWF
>> > private var urlLoader:flash.net.URLLoader;
>> >
>> >@@ -606,6 +618,7 @@ package org.apache.flex.net
>> > }
>> > }
>> >
>> >+
>> > if (_method !== HTTPConstants.GET &&
>> > !sawContentType && contentData) {
>> > element.setRequestHeader(
>> >
>>
>>
Re: git commit: [flex-asjs] [refs/heads/develop] - CORS security.
Allow auth credentials to be passed when using cross site calls. This is
required as well as setting the Access-Control-Allow-Origin header on the server.
Posted by Greg Dove <gr...@gmail.com>.
Just a general question about this type of thing:
Because this is JS-only, does GCC eliminate it as deadcode in a release
version if the method is never used? I would expect that, but I have not
checked that yet... if it does then perhaps the PAYG concern might be moot
(love that word!)
On Sun, Apr 9, 2017 at 4:48 PM, Alex Harui <ah...@adobe.com> wrote:
> IMO, with PAYG, this would go in an extension of HTTPService. Not all
> apps will need CORS.
>
> Thanks,
> -Alex
>
> On 4/8/17, 8:26 PM, "jmclean@apache.org" <jm...@apache.org> wrote:
>
> >Repository: flex-asjs
> >Updated Branches:
> > refs/heads/develop 11ef21aae -> 326d69791
> >
> >
> >CORS security. Allow auth credentials to be passed when using cross site
> >calls. This is required as well as setting the
> >Access-Control-Allow-Origin header on the server.
> >
> >
> >Project:
> >https://na01.safelinks.protection.outlook.com/?url=
> http%3A%2F%2Fgit-wip-us
> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Frepo&data=02%
> 7C01%7C%7C58994717190
> >044f003a908d47ef83ba7%7Cfa7b1b5a7b34438794aed2c178de
> cee1%7C0%7C0%7C6362730
> >52005920291&sdata=eQwxowk79ikXeDxbqXV3OeVrXUzTXN
> fFR0eKzhU8wiw%3D&reserved=
> >0
> >Commit:
> >https://na01.safelinks.protection.outlook.com/?url=
> http%3A%2F%2Fgit-wip-us
> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Fcommit%
> 2F326d6979&data=02%7C01%7C%
> >7C58994717190044f003a908d47ef83ba7%7Cfa7b1b5a7b34438794aed2c178de
> cee1%7C0%
> >7C0%7C636273052005930299&sdata=f8udpkPpLcL1ivRV3LDW0kJARc8QnL
> hBDHVFGgCko7M
> >%3D&reserved=0
> >Tree:
> >https://na01.safelinks.protection.outlook.com/?url=
> http%3A%2F%2Fgit-wip-us
> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Ftree%2F326d6979&
> data=02%7C01%7C%7C
> >58994717190044f003a908d47ef83ba7%7Cfa7b1b5a7b34438794aed2c178de
> cee1%7C0%7C
> >0%7C636273052005930299&sdata=t6MMifwasbm2bgCuzsVN2q4%
> 2BCYcB2uB8o7O%2B%2BJu
> >yZ5w%3D&reserved=0
> >Diff:
> >https://na01.safelinks.protection.outlook.com/?url=
> http%3A%2F%2Fgit-wip-us
> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Fdiff%2F326d6979&
> data=02%7C01%7C%7C
> >58994717190044f003a908d47ef83ba7%7Cfa7b1b5a7b34438794aed2c178de
> cee1%7C0%7C
> >0%7C636273052005930299&sdata=gJDrCW4YbwaorNFSXVCVjLyq3lwTC6
> 7VhCRLEtYlcD0%3
> >D&reserved=0
> >
> >Branch: refs/heads/develop
> >Commit: 326d69791b37cc2aaac546bcfcd3a51e88716f2f
> >Parents: 11ef21a
> >Author: Justin Mclean <jm...@apache.org>
> >Authored: Sun Apr 9 13:26:30 2017 +1000
> >Committer: Justin Mclean <jm...@apache.org>
> >Committed: Sun Apr 9 13:26:30 2017 +1000
> >
> >----------------------------------------------------------------------
> > .../src/main/flex/org/apache/flex/net/HTTPService.as | 13
> +++++++++++++
> > 1 file changed, 13 insertions(+)
> >----------------------------------------------------------------------
> >
> >
> >https://na01.safelinks.protection.outlook.com/?url=
> http%3A%2F%2Fgit-wip-us
> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Fblob%2F326d6979%
> 2Fframeworks%2Fpro
> >jects%2FNetwork%2Fsrc%2Fmain%2Fflex%2Forg%2Fapache%2Fflex%
> 2Fnet%2FHTTPServ
> >ice.as&data=02%7C01%7C%7C58994717190044f003a908d47ef8
> 3ba7%7Cfa7b1b5a7b3443
> >8794aed2c178decee1%7C0%7C0%7C636273052005930299&sdata=
> TcDMCOEVbLKxedpsnACV
> >OmZon89YgUkvGxOtd%2F3Qky8%3D&reserved=0
> >----------------------------------------------------------------------
> >diff --git
> >a/frameworks/projects/Network/src/main/flex/org/
> apache/flex/net/HTTPServic
> >e.as
> >b/frameworks/projects/Network/src/main/flex/org/
> apache/flex/net/HTTPServic
> >e.as
> >index b939751..3a9968c 100644
> >---
> >a/frameworks/projects/Network/src/main/flex/org/
> apache/flex/net/HTTPServic
> >e.as
> >+++
> >b/frameworks/projects/Network/src/main/flex/org/
> apache/flex/net/HTTPServic
> >e.as
> >@@ -505,6 +505,18 @@ package org.apache.flex.net
> > return null;
> > }
> >
> >+ /**
> >+ * Allows Javascript cross-site Access-Control requests to be
> >made
> >+ * using credentials such as cookies or authorization headers
> >+ *
> >+ * @productversion FlexJS 0.8
> >+ */
> >+ COMPILE::JS
> >+ public function set withCredentials(value:Boolean):void {
> >+ var element:XMLHttpRequest = this.element as XMLHttpRequest;
> >+ element.withCredentials = value;
> >+ }
> >+
> > COMPILE::SWF
> > private var urlLoader:flash.net.URLLoader;
> >
> >@@ -606,6 +618,7 @@ package org.apache.flex.net
> > }
> > }
> >
> >+
> > if (_method !== HTTPConstants.GET &&
> > !sawContentType && contentData) {
> > element.setRequestHeader(
> >
>
>